-
Notifications
You must be signed in to change notification settings - Fork 806
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
TeamInvitationController not using custom TeamInvitation model #890
Comments
Hmmm yeah it's probably best to use one of your two solutions. Tbh I think we can go for solution 2 since we also use the same solution like in the other PR. Feel free to attempt that PR and I'll back it up. Thanks |
This was referenced Sep 28, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description:
The accept and destroy methods in the TeamInvitationController use route model binding to inject the requested team invitation model. However, those instances are always of class
\Laravel\Jetstream\TeamInvitation
. The custom model class, e.g.\App\Models\TeamInvitation
is not used.Our application creates audit logs when models are being changed using an external package. Team invitations are audited as well so
\App\Models\TeamInvitation
implements interface and trait provided by the external package. As theTeamInvitationController
retrieves the wrong classes the "magic" will not happen when the model gets deleted.I am more than happy to send a pull request. But before I do so what are the chances of that PR to be merged? I provided a description for two of many possible solutions below. Do you think that this is a breaking change?
Steps To Reproduce:
dd(get_class($invitation));
here\Laravel\Jetstream\TeamInvitation
Solution 1
Override
resolveRouteBinding
to useJetstream::teamInvitationModel()
in the base TeamInvitation model.Downside: might not be fully backward compatible when projects created a custom team invitation model not inheriting from the base Jetstream TeamInvitation model.
Solution 2
Remove the the type-hint from the controller methods and query manually, see #651.
Downside: again not fully sure about backward compatibility when projects inherit from Jetstream base controller and override the methods mentioned before. All of sudden the base class would not have a type-hint anymore.
The text was updated successfully, but these errors were encountered: