Fix duping for AR KeyValue backend - #126
Conversation
| super(source) | ||
| self.send("#{association_name}=", source.send(association_name).map(&:dup)) | ||
| # Set inverse on associations | ||
| send(association_name).each { |translation| translation.translatable = self } |
There was a problem hiding this comment.
If I leave out the spec checking that changing the attribute on one instance does not affect the value on the other instance, then this can be simplified to just:
define_method :initialize_dup do |source|
super(source)
self.send("#{association_name}=", source.send(association_name))
endi.e. you don't need to dup the association objects, nor set the inverse.
However, I personally think that it's better that the dup'ed instance does not affect the original and vice versa. However, Sequel (for example) does not "deep dup" for any pattern here used in the backends... I suppose because there's a performance cost, and Sequel always defaults to not incurring any performance costs.
There was a problem hiding this comment.
Never mind, we definitely want duplicate models to have their own translations with AR at least. I think this is the correct way to fix this.
1 participant
Fixes #123