ruby-hyperloop / hyper-mesh

The project has moved to Hyperstack!! - Synchronization of active record models across multiple clients using Pusher, ActionCable, or Polling
https://hyperstack.org/
MIT License
22 stars 12 forks source link

Errors aren't cleared if you don't save because a record hasn't "changed" #54

Open sfcgeorge opened 6 years ago

sfcgeorge commented 6 years ago

Then if you change the attribute to fix the validation error but the change happens to put the record back to its original unchanged state then the errors aren't cleared because no second save ever occurs. So the record appears to be invalid when it's not, and you can't save it because it hasn't "changed".

# load some existing record
record = Record.all.first
# assume a validation means the foo attribute must be true
puts record.foo # => true

# change an attribute making it invalid
record.foo = nil 

# changed? is true, save attempted, fails due to validation as expected
record.save if record.changed? 

# prints the error as expected
puts record.errors

# restore attribute to valid value
record.foo = true

# record is now not changed from fetched values so no save
record.save if record.changed? 

# prints the old error even though we've now fixed it!
puts record.errors

I can see why this happens, errors are added / cleared on save, so a save has to occur for errors to be cleared. Why is this a problem? I have code that saves when you click "Next Page", and if saving fails doesn't let you go to the next page. But I only save a record if its changed, so you end up not being able to move on because you're stuck between the error and changed states. The following code works as expected because I'm checking for changed and errors, but previously I was just checking for errors which is how it got stuck.

def continue # called by a button with a `.then` which moves to the next page
  records = profile.degrees.map do |r|
    r.save if r.changed?
  end
  Promise.when(*records).then do
    if profile.degrees.any? { |x| x.changed? && x.errors&.any? }
      Promise.new.reject
    else
      Promise.new.resolve
    end
  end
end

I'm not sure if this is an issue or not as the current behaviour kinda makes sense, maybe just something to be aware of.

catmando commented 6 years ago

Good catch