active-record
active-record copied to clipboard
[2.1][Discussion] ActiveRecord::link behavior
The behavior of link() doesn't feel intuitive and in some cases even faulty to me.
Consider I have models A, B and C. Table for model A looks like this:
id (pk)
b_id (int) --> foreign key constraint on B
c_id (int) --> foreign key constraint on C
f1 (varchar)
f2 (int)
Now I'm creating a new model A:
$b = ..;
$c = ..;
$a = new A();
$a->load(...);
$a->link('relationB', $b);
$a->link('relationC', $c);
$a->save();
The reason I want to use link is that my model defines how the relationship works, code using the model doesn't care or know about field names, it just knows about relation names.
The code above will not work, since the first call to link will cause an exception because of the foreign key constraint on c_id.
The final call to $a->save() doesn't really do anything since link already takes care of saving.
I'm not sure what the best approach forward is, but currently using link() requires a very detailed knowledge of the underlying models that kind of makes the function useless in scenarios where I can just set the foreign key manually on the newly created object.
One possible simplification would be that link() never saves new records.
Consider: $a->link('relationB', $b).
The behavior would then be:
$b->isNewRecord() ? exception
$a->isNewRecord() ? update field b_id, no database write.
!$a->isNewRecord() ? update field b_id, only save field b_id.
// Foreign key a_id in b instead.
$a->isNewRecord() ? exception
$b->isNewRecord() ? update field a_id, no database write.
!$a->isNewRecord() ? update field a_id, only save field a_id.
This still suffers from the problem that the caller needs to know details that should not concern him.
$a->link('b', $b') should always be the same as $b->link('a', $a).
A possible but complex solution would be to use event handlers to figure this out after the models are saved, this could be troublesome with foreign keys in case models are not saved in the right order.
I'd like to get a discussion going on possible alternative solutions for 2.1.
related: https://github.com/yiisoft/yii2/issues/12147, https://github.com/yiisoft/yii2/issues/4834
IMO saving the model by bindModels() is not expected at all and leads to problems, only one of which is described here. I posted an extended use case in this related issue https://github.com/yiisoft/yii2/issues/12147#issuecomment-371069651.