Skip to content

[10.x] Database layer fixes#49787

Merged
taylorotwell merged 1 commit intolaravel:10.xfrom
saadsidqui:database_layer_fixes
Apr 15, 2024
Merged

[10.x] Database layer fixes#49787
taylorotwell merged 1 commit intolaravel:10.xfrom
saadsidqui:database_layer_fixes

Conversation

@saadsidqui
Copy link
Contributor

@saadsidqui saadsidqui commented Jan 23, 2024

This PR fixes a couple of issues discussed in #49735 .

Appropriate checks are added to Illuminate\Database\Eloquent\Relations\BelongsTo::getForeignKeyFrom() and Illuminate\Database\Query\Builder::whereIntegerInRaw() to return the proper value if a BackedEnum is encountered. This is typically the case on models where a cast has been defined, especially on keys.

I have also added a couple of tests to cover these cases where it seemed appropriate.

There might be other similar cases. Please see this comment.

@github-actions
Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

… BackedEnums

- Illuminate\Database\Eloquent\Relations\BelongsTo::getForeignKeyFrom() handles BackedEnums properly

See #49735
@saadsidqui saadsidqui marked this pull request as ready for review January 23, 2024 02:54
@driesvints driesvints changed the title Database layer fixes [10.x] Database layer fixes Jan 23, 2024
Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all I can see nothing wrong with this PR. Thanks 👍

@taylorotwell taylorotwell marked this pull request as draft January 24, 2024 15:05
@saadsidqui
Copy link
Contributor Author

@taylorotwell sorry I'm confused as to why the PR was reverted to a draft. If there are any comments or suggestions I'll be happy to contribute the fixes.

@saadsidqui
Copy link
Contributor Author

@driesvints any idea what else is needed here? I'm very confused.

@driesvints
Copy link
Member

Hi @sidquisaad. Most likely Taylor is too busy rn to review and will review at a later moment. Thanks!

@driesvints driesvints marked this pull request as ready for review April 11, 2024 07:26
@driesvints
Copy link
Member

@taylorotwell gonna mark this ready for you again if that's okay.

@taylorotwell taylorotwell merged commit 192f909 into laravel:10.x Apr 15, 2024
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