Inconsistency between required and one_of

[tpow]

In #45 there was discussion about the difference between required and exists. Required means that the field exists and isn't empty.

In the documentation for validation, one_of is described as "Sometimes you will want only one of several fields to be required. At least one of them need to be required." This sounds like what I want -- I have a couple fields and I want the user to populate one of them and leave the other one blank (or optionally populate both).

However, in my testing with form input, one_of seems to be acting like it is testing that the fields exist but isn't testing that one of them is required. If I validate.required(["field1", "field2"]) it throws an error for both fields. However, validate.one_of(["field1", "field2"]) throws no errors.

If one_of is intentionally for exists_one_of, it would be helpful to have a required_one_of (or some other name). The documentation also needs to be clarified.

[tpow]

In case it is helpful, I ended up with this for a required_one_of:

class required_one_of(BaseValidation):                                           
    def passes(self, attribute, key, dictionary):                                
        # Only check this on the last key in the validations                     
        # This avoids a duplicate message for each validation item               
        if key != self.validations[-1]:                                          
            return True  # If not the last of the list, say it worked            
        for validation in self.validations:                                      
            if self.find(validation, dictionary):                                
                return True                                                      
                                                                                 
        return False                                                             
                                                                                 
    def message(self, attribute):                                                
        if len(self.validations) > 2:                                            
            text = ", ".join(self.validations)                                   
        else:                                                                    
            text = " or ".join(self.validations)                                 
                                                                                 
        return "The {} is required.".format(text)                                
                                                                                 
    def negated_message(self, attribute):                                        
        if len(self.validations) > 2:                                            
            text = ", ".join(self.validations)                                   
        else:                                                                    
            text = " or ".join(self.validations)                                 
                                                                                 
        return "The {} is not required.".format(text)

I'm not sure if I am doing something wrong or if one_of normally gives a duplicate message for each field. I wasn't happy with this, so I made this required_one_of only check on the last key. This also theoretically (haven't tried) supports the * syntax like required because it uses the same find function.

[girardinsamuel]

@tpow
You're right, one_of is intentionally for exists_one_of even if the word required appears in documentation ... but last sentence suggests that it's only a key presence check : This will pass because at least 1 value has been found

But I agree, the doc should be clarified here and why not add the equivalent of one_of for required fields 😉
Could you translate your code snippet as a PR targeting 3.0 ? that would be awesome

[girardinsamuel]

I'm not sure if I am doing something wrong or if one_of normally gives a duplicate message for each field. I wasn't happy with this, so I made this required_one_of only check on the last key. This also theoretically (haven't tried) supports the * syntax like required because it uses the same find function.

Ideally that's a good occasion to add unit tests for those cases

Feel free to ask if you need help about that