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

Fix Model->isRelation Method to Accurately Identify Relationships #52520

Draft
wants to merge 2 commits into
base: 11.x
Choose a base branch
from

Conversation

o-kima
Copy link
Contributor

@o-kima o-kima commented Aug 19, 2024

This PR addresses an issue in the isRelation method of the HasAttributes trait, where the method would incorrectly return true for any key that exists as a method on the model. This caused issues when determining whether a given key represents a relation.

Previously, isRelation could mistakenly identify non-relationship methods as relationships, leading to unexpected behavior in Eloquent models. This fix ensures that only methods returning a Relation subclass are treated as relations, improving the reliability and accuracy of relationship handling in Eloquent models.

@o-kima o-kima changed the title Bugfix/model is relation Fix Model->isRelation Method to Accurately Identify Relationships Aug 19, 2024
@rodrigopedra
Copy link
Contributor

rodrigopedra commented Aug 19, 2024

This will only work if a developer defines a return type for all of their relation methods. Which isn't enforced neither by Laravel, nor by PHP.

Therefore, it is a breaking change, a huge one, and might be the reason that so many tests are failing.

If there is a defined return type, I agree we could have this additional check. But for methods that don't have a defined return type, the old behavior should be kept.

@crynobone
Copy link
Member

Setting this to draft until tests are passing (and without introducing any breaking changes)

@crynobone crynobone marked this pull request as draft August 20, 2024 02:01
@MahdiSaremi
Copy link

These changes will break the production programs! With this change, the return type must be defined in all relations.

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.

4 participants