extension/learn — comment on commit 7844c39222f3b563a6257f46558288139cdd2690
File: app/models/learner.rb line 103
By jasonadamyoung:

This code isn't going to work - and may have unintended side effects that includes damaging the data in associated objects.

Take the last two associations in Learner that were committed last tuesday and wednesday (that you just merged yesterday on the conflict merge)

has_many :presented_events, through: :presenter_connections, source: :event
has_many :preferences, :as => :prefable

In the code above, you'll loop through a set of Events and set event.learner = learner_to_keep. That isn't going to work. Because Event doesn't belong_to learner - and even if it did, it wouldn't be this learner at all (and you would have just set the wrong data). This is a case where you have to operate on the join table.

In the second, you are looping through Preferences and setting preference.learner = learner_to_keep - same deal, it doesn't belong_to learner. It's a polymorphic association with prefable_id

Finally, take one that's missing, but should be there - the creator association for Events

if you were to have

has_many :created_events, :class_name => 'Event', :foreign_key => 'creator_id'

you have a similar situation to the first, you'll be setting event.learner - which doesn't exist (and if it did, it'd be wrong) - that's a case where you need to be setting event.creator

I think it's debatable here about using .save to update _id's associated with learners. The issue is going to be triggering all the callbacks that may exist on the objects - which may or may not be desired (Event#save for example triggers solr callbacks - it's likely to trigger notification callbacks, etc.) - this may be a case where doing update_all is desired, not only for the performance, but to avoid side-effect callbacks you weren't expecting - and you explicitly trigger callbacks you know have to happen from a _id's change for learners.

I also think the 'chuck' colloquialism is less clear than remove or destroy (e.g. learner_to_remove). But that's ticky-tack on my part.