Associations automatically saving when using attributes= and assign_attributes()

[leehericks]

According to Neo4j.rb documentation here, setting associations saves automatically.

In Rails 5, AttributeAssignment was moved from ActiveRecord to ActiveModel as mentioned here.

Therefore, ActiveNode has the following methods:

model.assign_attributes()
model.attributes=      # An alias for assign_attributes()

The problem is that assign_attributes() and attributes= are not supposed to save or call any validations. Good Rails 3/4 usage explanation here

In my Rails 5 project, using mass assignment from a form causes the associations to persist immediately, which causes issues with setting attributes and checking for changes as seen in the following code:

def update
  @kanji = Kanji.find(params[:id])
  @kanji.assign_attributes(kanji_params)     # <- Here associations save automatically

  if @kanji.changed?
    if @kanji.save
      flash[:success] = "Kanji updated successfully."    # <- Not set if the association was the only value changed on the form, therefore save returned false
    else
      flash[:error] = "One or more errors occured."
    end
  end

  redirect_to edit_kanji_path(@kanji)
end

private

def kanji_params
  params.require(:kanji).permit(:strokes, :skip_code, :kanken_level_id, jlpt_level_ids: [])
end

Of course update_attributes() is documented to run validations and save the model and this is where associations persisting is acceptable and works correctly.

Rails 5.1.4, Neo4j.rb 9.0.7, Neo4j 3.3.0

[cheerfulstoic]

Firstly, sorry for not responding sooner!

I didn't actually know about assign_attributes in ActiveRecord or ActiveModel, but I'm always happy to have more things in ActiveModel that we can take advantage of.

When you mentioned assign_attributes, I wondered if it was calling #write_attribute for each key / value pair or if it was calling the #key= method. Looking at the code, it seems to be the latter:

https://github.com/rails/rails/blob/e57d8d090d5d6860e3a5bb3257d6aed5b644ea25/activemodel/lib/active_model/attribute_assignment.rb

Also, since I was unfamiliar with assign_attributes, I didn't realize that the neo4j gem was defining it (see this bit of code). The code looks to be about the same, so we could probably drop our implementation and replace it with include ActiveModel::AttributeAssignment. You could simulate that for now with:

Neo4j::Shared::MassAssignment.send(:remove_method, :assign_attributes)

class YourModel
  include ActiveModel::AttributeAssignment
end

I suspect that won't fix your issue, but it's a good place to start to figure this out.

Actually... Looking at the neo4j implementation I'm noticing that if the attribute isn't defined we call add_undeclared_property which has an empty definition unless you include Neo4j::UndeclaredProperties. So that could be tricky... Heads up to @klobuczek since he put in the UndeclaredProperties module. Perhaps it makes sense for us to still include ActiveModel::AttributeAssignment but provide our own implementation that calls super and rescues from ActiveModel::UnknownAttributeError

Ultimately, though, if there's still a problem I think it might not be the fault of assign_attributes but rather the association code since assign_attributes is really just calling association=. If this acts differently than ActiveRecord then we should definitely investigate it (though it would be a breaking change, and potentially a big one)

[leehericks]

Thanks for investigating this.

Yes, it seems simply to boil down to associations being saved automatically, which is not always desirable. I may wish to assign the properties and associations and call valid? for instance. If there were some errors in my form for properties, of course, I would't want those associations created yet because I will reshow the form with errors and it is unknown if the user will correct the form and submit again.

I think assign_attributes and assignment in general offers the ability to run validations, examine the dirty state of the object and conditionally persist or take some other actions, so I can only think of two ways to correct this:

1. Remove this automatic saving when assigning to associations.
2. Add an option to the association to delay persistence

In the case of (1), I personally think of properties and associations the same way. The are assignable and accessible values. In this case I don't think their persistence model should differ, ESPECIALLY since they get assigned together through the attributes methods.

As for (2), it is a non-breaking solution but again something that Rails programmers might not be used to. Perhaps has_many :out, :things, persist_on_assignment: false because ActiveRecord has the option :autosave which may be confusing.

[cheerfulstoic]

:+1: I think if we had associations automatically saving it was because we thought that that's what ActiveRecord did. I'm happy to take their guidance on how to implement this whenever it makes sense for Neo4j

[SJTucker]

I'm running into this issue as well. Any word on this being addressed?

[jboler]

@klobuczek @amitsuryavanshi I vote for prioritizing this one.