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

Can't save tree of records with association presence validation #58

Open sfcgeorge opened 6 years ago

sfcgeorge commented 6 years ago

I have a model School that has_many :subjects, class_name: "SchoolSubject" and a validates_presence_of :subjects. It is impossible to save via Hyperloop! In the old Rails form using accept_nested_attributes_for it works fine because they are nested and Rails is smart to save them in such a way that the validation works. But I guess Hyperloop doesn't do this, or doesn't set up the association, or something, because I get the error that subject is missing 😬

Request:

{"operation":"ReactiveRecord::Operations::Save", "params":{"models":[{"id":774254, "model":"SchoolSubject", "attributes":{"id":null, "school_qualification_id":null, "school_grade_id":null, "name":"Maths"}, "vector":["SchoolSubject", ["new", 774254]]}, {"id":774250, "model":"School", "attributes":{"id":null, "from":"1973-01-01", "to":"1979-01-01", "name":"My School", "city":"London", "country":"England"}, "vector":["School", ["new", 774250]]}, {"id":8080, "model":"CandidateProfile", "attributes":{"id":3}, "vector":["CandidateProfile", ["find_by_id", 3]]}, {"id":8098, "model":"School", "attributes":{"id":3}, "vector":["School", ["find_by_id", 3]]}], "associations":[{"parent_id":774254, "attribute":"school", "child_id":774250}, {"parent_id":774250, "attribute":"profile", "child_id":8080}, {"parent_id":774250, "attribute":"subjects", "child_id":774254}, {"parent_id":8080, "attribute":"schools", "child_id":8098}, {"parent_id":8080, "attribute":"schools", "child_id":774250}], "validate":true}}

Response:

{"response":{"success":false,"saved_models":[[774254,"SchoolSubject",{"id":108,"school_id":null,"school_qualification_id":null,"school_grade_id":null,"name":"Maths","created_at":"2018-02-08T19:20:11.951+00:00","updated_at":"2018-02-08T19:20:11.951+00:00","original_id":null,"info_sources":null},null],[774250,"School",{"id":null,"profile_id":3,"name":"My School","city":"London","country":"England","from":"1973-01-01","to":"1979-01-01","created_at":null,"updated_at":null,"original_id":null,"info_sources":null},{"subjects":["can't be blank"]}]],"message":"HyperModel saving records failed!"}}

Struggling to think of any workaround other than removing the validation. I had a similar issue with a maximum associated records validation, but I was able to move that to the associated record. That won't work here though as the only time you want the validation to run is when there isn't an associated, in which case the validation wouldn't be there to run.

Moving a maximum validation into the associated model works but won't work for minimum:

class IndustriesPreference < ApplicationRecord
  belongs_to :industry
  belongs_to :preference, touch: true

  validate :maximum_industries

  private

  def maximum_industries
    return unless preference.industries.count >= 3
    errors.add(:industries, "maximum 3 industries")
  end
end

I think in Hypermesh the validations need to happen at the end of the transaction: save all records ignoring validations, then call valid? on all records that were saved and if any aren't then rollback the transaction.

sfcgeorge commented 6 years ago

Ok I found a way to work around but it's pretty gross:

class School < ApplicationRecord
  has_many :subjects, class_name: "SchoolSubject", dependent: :destroy

  validates_presence_of :name, :country, :from, :to, :city

  after_save do
    next unless subjects.empty?
    errors.add(:subjects, "can't be blank")
    raise ActiveRecord::Rollback
  end
end
catmando commented 6 years ago

At first glance it seems like the key line has been elided:

https://github.com/ruby-hyperloop/hyper-mesh/blob/ebf661eecc592799b7723bb666c1d42d84551918/lib/reactive_record/active_record/reactive_record/isomorphic_base.rb#L420

We will have to re-enable that line and if it causes problems, find an intelligent solution.

I am figuring in some review of the code I thought: Hey this is unnecessary since it will hit the child record eventually and save that, which without the validates_presence_of works. Hopefully was just going for effeciency :-)