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

Fixed support for HasOne relationships when sorting #12

Merged

Conversation

kevinwheeler
Copy link
Contributor

In the original code, line 128 is as follows:
$ownerKey = $relationship->getOwnerKeyName();

However, HasOne relationship instances don't have a getOwnerKeyName method.

@kevinwheeler kevinwheeler marked this pull request as ready for review November 19, 2022 15:33
@kevinwheeler
Copy link
Contributor Author

kevinwheeler commented Nov 19, 2022

I have only thought through the BelongsTo and HasOne cases. Those were the only cases included in the original code before I modified it anyways. Perhaps my changes also fix BelongsToMany relationships, but I haven't looked into it.

Adding support for HasMany relationships may be as simple as changing
if ($relationship instanceof HasOne) {
on line 131 to
`if ($relationship instanceof HasOne || $relationship instanceof HasMany ) {

and importing HasMany.

I'm unlikely to spend time thinking through if this would work, but we could always add this line of code anyways with a comment that caveats that it may not work. Let me know if you would like me to do this.

@coolsam726 coolsam726 merged commit b7588cc into savannabits:main Dec 30, 2022
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.

2 participants