Skip to content

Dependency doesn't play well with required#665

Merged
nicolaiarocci merged 1 commit into
pyeve:developfrom
rs:dependencies_with_required
Jul 7, 2015
Merged

Dependency doesn't play well with required#665
nicolaiarocci merged 1 commit into
pyeve:developfrom
rs:dependencies_with_required

Conversation

@rs

@rs rs commented Jul 2, 2015

Copy link
Copy Markdown
Collaborator

If a field has both a dependency and a required directive, the required directive is just ignored. This PR contains a failing test to demonstrate the issue.

@nicolaiarocci

Copy link
Copy Markdown
Member

@rs

rs commented Jul 2, 2015

Copy link
Copy Markdown
Collaborator Author

In my case the dependency is met thru the default value of the depending field but still doesn’t complain if the required field is omitted. I can change the test to meet that case if you want.

Also I think that when you use required, you expect that whatever the other directives, you won’t end up with a missing field in the db.

@nicolaiarocci

Copy link
Copy Markdown
Member

Also I think that when you use required, you expect that whatever the other directives, you won’t end up with a missing field in the db.

it used to be like this. New behaviour was introduced with pyeve/cerberus/pull/61 by @hthieu1110. This test in particular shows that this is in fact the intended behaviour.

Now, I agree that this is somewhat convoluted. required being validated only when dependencies are met seems counter-intuitive. In this scenario it seems appropriate to give precedence to the required rule over dependencies.

I'd appreciate if @hthieu1110, as the original patch author, would chime in on this one.

nicolaiarocci referenced this pull request in pyeve/cerberus Jul 7, 2015
'required' is always evaluated, independent of eventual missing
dependenies. This changes the previous behaviour whereas a required
field with dependencies would only be reported as missing if all
dependencies were met. A missing required field will always be reported.

Addresses nicolaiarocci/eve#665.
nicolaiarocci referenced this pull request in pyeve/cerberus Jul 7, 2015
'required' is always evaluated, independent of eventual missing
dependencies. This changes the previous behaviour whereas a required
field with dependencies would only be reported as missing if all
dependencies were met. A missing required field will always be reported.

Addresses nicolaiarocci/eve#665.
@nicolaiarocci nicolaiarocci merged commit 724fe56 into pyeve:develop Jul 7, 2015
@gmauleon

gmauleon commented Sep 8, 2015

Copy link
Copy Markdown

Sorry to jump in so late, just starting to use Eve. But then how could I require a field only if another field is of value X?

I got a field "mytype" with 2 possible value A and B, I want that when "mytype" is A only, then field "myfield" is required. I did that with a dependency set on the field "myfield" that need "mytype" to be A. But this patch broke that logic. Any workaround ?

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants