Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate_presence should work for date fields #91

Open
robj opened this issue Dec 5, 2013 · 14 comments
Open

validate_presence should work for date fields #91

robj opened this issue Dec 5, 2013 · 14 comments

Comments

@robj
Copy link
Contributor

robj commented Dec 5, 2013

It is not immediately apparent from docs it only supports Numeric/String

my custom validator for the interim:

def validate_date_presence(field, value, setting)
  if(value.is_a?(Time || Date))
    return true
  else
    return false
  end
end
@sxross
Copy link
Owner

sxross commented Dec 5, 2013

In what case are you expecting an invalid date? Aren't you using a date picker?

@robj
Copy link
Contributor Author

robj commented Dec 5, 2013

In this case I am creating models from a JSON API. Validations are to ensure I only have valid models even in the case something is not correct server side (ie. I get sent a nil date)

@sxross
Copy link
Owner

sxross commented Dec 5, 2013

IIRC, you should be able to use before_validation(sender) for this. You may disagree with this approach, but take it out for a spin.

def before_validation(sender)
  !sender.create_date.nil?
end

or something like that. Your validation is equally valid and if you would like to submit a pull request with a spec, that would be great.

@robj
Copy link
Contributor Author

robj commented Dec 5, 2013

Thanks, As a simple change to validate_presence, would this suffice?

def validate_presence(field, value, setting)
if(value.is_a?(Numeric || Time || Date))
return true
elsif value.is_a?(String) || value.nil?

(or just including Time, as I presume all date casts are to Time?)

@sxross
Copy link
Owner

sxross commented Dec 5, 2013

Let's take a step back. Say there are two ways of populating your model. Way 1 is that the user enters info on their iPhone. Further, say that your model contains start time and end time, so it's ok to start something without ending it. That implies that for Way 1, a nil date is ok. Way 2 is to slurp it off the server, in which case, nil dates are not ok.

So the default "validate this way all the time" ala Rails is not actually perfect for this kind of thing. I don't know if you have this requirement, but it's something worth considering.

Comments?

@robj
Copy link
Contributor Author

robj commented Dec 5, 2013

I'd say in the case a nil date is OK, then it's counterintuitive to use a presence validator at all?

On 6 Dec 2013, at 9:02 am, "s.ross" notifications@github.com wrote:

Let's take a step back. Say there are two ways of populating your model. Way 1 is that the user enters info on their iPhone. Further, say that your model contains start time and end time, so it's ok to start something without ending it. That implies that for Way 1, a nil date is ok. Way 2 is to slurp it off the server, in which case, nil dates are not ok.

So the default "validate this way all the time" ala Rails is not actually perfect for this kind of thing. I don't know if you have this requirement, but it's something worth considering.

Comments?


Reply to this email directly or view it on GitHub.

@DougPuchalski
Copy link
Contributor

@robj In Rails that would be validates_presence_of :created_at, allow_nil: true, to disallow blanks. If you want to enforce a date format or a parseable date format, that would be a custom validator and possibly an overridden setter method that parses dates if they are strings inbound.

@robj
Copy link
Contributor Author

robj commented Dec 5, 2013

currently validates_presence fails validation if the field is not of a numeric or string type. This seems to violate the principle of least surprise if it is not usable on all available field types.

On 6 Dec 2013, at 10:39 am, Doug Puchalski notifications@github.com wrote:

@robj In Rails that would be validates_presence_of :created_at, allow_nil: true, to disallow blanks. If you want to enforce a date format or a parseable date format, that would be a custom validator and possibly an overridden setter method that parses dates if they are strings inbound.


Reply to this email directly or view it on GitHub.

@sxross
Copy link
Owner

sxross commented Dec 6, 2013

@robj You might want to use a format validator. From the spec:

validate      :some_day, :format => /\A\d?\d-\d?\d-\d\d\Z/

This would fail if nil and only succeed if the date were formatted as specified (you would probably want to use ISO dates, judging from your earlier issue).

I know you've already read this, but the relevant portion of the documentation regarding custom validators is:

https://github.com/sxross/MotionModel#validation-methods

@pixlwave
Copy link
Contributor

This has just had me going round in circles too until I decided to remove my validations. I tried validating the presence of a date and an array, both of which failed even though they were present. I was going to call the following line on each field change in my ViewController to disable the save button until all the required fields were filled in.
saveButton.enabled = myNewObject.valid?
This seems like sensible logic to me?

@sxross
Copy link
Owner

sxross commented Mar 18, 2015

I'm not sure this will work. The object does not have any data until you put it there. If you aren't doing that as the user enters it, you can't validate against it. Further, once you assign to a model field of type date or type array, MotionModel casts the value to it's Ruby type. So, presence validates against nil or length == 0. The latter case may work for array but not for date. This may not be a model issue.

@pixlwave
Copy link
Contributor

Ok so in which case, if I'm migrating some code from CDQ, I'd implement datetime :mydate, optional: false with a format validation as shown above? And for an array, I probably need to make a validation with a block of some sort right?

@sxross
Copy link
Owner

sxross commented Mar 18, 2015

I'm not sure about the optional: part. I think I'd go with default: if you want to set it to something. If you are migrating from CDQ will you have a string or an NSDate. MotionModel will recognize NSDate and copy it over. If it's a string, it will try to use a NSDataDetector to parse it.

For the array, presence: looks at the length. Once you have assigned the array to your model, it will know the length and be able to validate. So you could as easily say: length: [4..12] or something like that.

Remember, validations are not necessarily for doing on-the-fly form validations -- more so to ensure data integrity. There's no reason not to use them for input validations, but don't forget to roll back your transaction before moving on if the user cancels.

The main place I'm looking is motion/validatable.rb. It's pretty straightforward code.

@pixlwave
Copy link
Contributor

Yep I've not an NSDate, so everything is easy there. Thanks for the help, this makes more sense now :)

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

No branches or pull requests

4 participants