CODEWITHSUNDEEP
ruby-on-railsruby
acco
acco

Reputation: 550

Errors don't prevent object from saving?

I have a virtual attribute that takes a time range from a form field and splits it: 

def time_range=(time_range)
  unless time_range.empty?
    t = time_range.split(/to|\-/)
    self.start_entry = t[0]
    self.finish_entry = t[1]
    if Chronic.parse(self.start_entry).nil? || Chronic.parse(self.finish_entry).nil?
      errors.add(:time_range, 'Invalid time range entered')
    end
  end
end

start_entry and finish_entry are also virtual attributes as I have other ways to set them. Regardless of how the two were set, I have the following hook to set start and finish in my database:

before_save :set_start_and_finish

Despite the fact that I add an error, erroneous objects still manage to save:

> t = Tour.new
> t.time_range = "rubbish"
> t.errors
#=> {:time_range=>["Invalid time range entered"]}
> t.valid?
#=> true

How can I invalidate the instance to prevent it from saving later?

Upvotes: 3

Views: 2855

Answers (3)

mu is too short
mu is too short

Reputation: 434665

Calling t.valid? will clear the errors before running the validations so your validations inside time_range= are ignored.

If we look at ActiveRecords's valid?, we see this:

def valid?(context = nil)
  context ||= (new_record? ? :create : :update)
  output = super(context)
  #...

And the super should send you into ActiveModel's valid? which starts like this:

def valid?(context = nil)
  current_context, self.validation_context = validation_context, context
  errors.clear
  #...

and the clear call nukes the errors you added in time_range=.

If you want to validate something, use the validators. If you want to prevent an invalid assignment, raise an ArgumentError (or other more appropriate exception).

Having the validation system reset itself (i.e. errors.clear) before running the validations does make sense. If it didn't reset, you'd have to throw away and reload an invalid object (or manually reset it) just to correct a validation error. Just because "update, validate, save or destroy" is the general workflow for a web-app doesn't mean that it is the only possible workflow for a database-backed application.

Upvotes: 15

Pravin
Pravin

Reputation: 6662

Try calling set_start_and_finish on validate instead of before_save
As validate :set_start_and_finish

Upvotes: 0

jdl
jdl

Reputation: 17790

set_start_and_finish seems like an odd place to be checking for validation errors, but make sure that you are returning false if you detect an error to stop the rest of the callbacks from executing.

Read the section on "Cancelling callbacks"

Upvotes: 1

Related Questions