trailblazer / roar-rails

Use Roar's representers in Rails.
http://roar.apotomo.de
MIT License
235 stars 65 forks source link

Validation failures when modifying collections #100

Open elliot-nelson opened 9 years ago

elliot-nelson commented 9 years ago

In our API, one of the primary models has several sub-model collections. These sub models all have validations, and the primary model also validates various aspects of the relationship between the collections. (For example: you can't save the Order if there isn't at least one Item. You can't have more Items than item_limit. Etc.)

When creating a record using representers, everything goes smoothly. However, when updating a record, these validations don't really work: even when using the find_or_instantiate strategy, it seems like the core call (somewhere along the line) is:

order.items = [new array of items]

Unfortunately, on an existing record, this is an instant call-to-action by Rails -- it runs off and inserts and deletes records, all before save is ever called on the parent object.

This behavior by Rails is one reason all our web forms use nested_attributes_for: the web forms are submitting the "items_attributes" collection, which means Rails doesn't attempt any database modifications until all the models are validated.

I saw another ticket (https://github.com/apotonick/roar-rails/issues/81) where you mentioned that you shouldn't need to use nested_attributes with roar. Do you have any patterns that would allow roar to behave the way way nested_attributes does? (That is: that will not make any database updates until all of the models being touched have been validated.)

elliot-nelson commented 9 years ago

As I play more with this, I think I can get pretty close to the behavior I'm looking for using the options parameters. Here's my current draft that is very close to what I want:

collection :sites, class: Site, extend: SiteRepresenter,
  instance: lambda {|hash, *| new_record? ? sites.build : (sites.select {|site| site.id == hash["id"]}.first || sites.build)},
  setter: lambda {|value, *| self.sites = value if new_record?}

It looks like what I probably want to do is continue to tune these blocks until the behavior is what I want, then I can encapsulate it as a Strategy and use it on all the collections in the object.

(Note the key difference between this and the existing find_or_initialize strategy is that I'm grabbing references to objects in memory, attached to the main object. That way the main object will validate and save the entire object tree.)

apotonick commented 9 years ago

First of all: HAPPY NEW YEAR! :fireworks:

Currently, Representable compiles the entire collection and then assigns it to the model. We will add another semantic to Representable, but I advise you to remove validations from your models and move them into Reform forms (or, contracts).

This works exactly like Roar/Representable but deserialises to object graph and wraps it with Reform form objects, validations can run without writing to the model/database, then, once you're happy, you can sync the state to the persistence layer.

Have you seen my Trailblazer book? This is all part of my Trailblazer architecture, you should have a look.