Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Add the pivot's related model when creating from attributes #53694

Merged
merged 3 commits into from
Dec 2, 2024

Conversation

alexwass-lr
Copy link
Contributor

Problem

For advanced model relationships where a custom pivot model has been created, the developer has the opportunity to define their own fromRawAttributes method.

Within this they may wish to conduct certain actions depending on the relationship. For me, this is for the purpose of auditing.

Currently I am auditing all models through their events, including the pivots when extending the above class. However when I attach, detach, or updateExistingPivot, I can only identify the parent with $pivot->pivotParent. There is no way of knowing which relationship was called on the parent model so you don't know what was attached/detached/updated (which isn't ideal for an audit).

For example:

use App\Models\User;
 
$user = User::find(1);
 
$user->roles()->attach($roleId);

$pivot->pivotParent will be the User model, but the Pivot model doesn't know it's the Role model being attached.

Whilst the pivot keys are set (so we have role_id and user_id), there is no reliable way to identify the pivot's "child" (related) as the column names could potentially not follow convention (e.g. parent_role and user).

As $related is only calculated on the relationship, you could not handle this in fromRawAttributes without overwriting all the core model methods that call it, which will increase upgrade friction.

Solution

As we're already setting the pivotParent, I'm proposing we also set the related model (pivotRelated) on the Pivot. Even though the relationship builds this as an empty class, it still helps developers reliably get the accurate class. This could easily be combined with $pivot->getAttribute($pivot->getRelatedKey()) to get the model's ID, then a fresh instance if required.

Whilst I would prefer the property order to be slightly different so $related followed $parent, this solution felt the least intrusive option so we can retain backwards compatibility and not introduce any breaking changes.

Happy to look at alternative options though if I'm missing a simpler solution!

@taylorotwell taylorotwell merged commit e9dc1c4 into laravel:11.x Dec 2, 2024
38 checks passed
@alexwass-lr
Copy link
Contributor Author

Thanks!

@alexwass-lr alexwass-lr deleted the feat-pivot-relationships branch December 2, 2024 17:01
@ralphjsmit
Copy link
Contributor

Totally love this, I actually had the need recently as well for this. Worked around with it using reflection and checking for which BelongsTo relationship matches the relatedKey. Thanks for PR'ing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants