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: use qualifyColumn rather than assuming format #53559

Merged
merged 3 commits into from
Nov 18, 2024

Conversation

willtj
Copy link
Contributor

@willtj willtj commented Nov 18, 2024

In my setup I have some Laravel models stored in MongoDB and others in MySQL. MorphTo and BelongsTo relations aren't loading where the parent model is in MongoDB and the child is in MySQL, i.e:

class Parent extends MongoModel
{

}

class Child extends EloquentModel
{
    public function myRelation()
    {
        return $this->belongsTo(Parent::class);
    }
}

Child::first()->myRelation will return null because addConstraints() is assuming that the key name should be prepended with the table name, which isn't the case for MongoDB.

The getQualifiedOwnerKeyName() method already exists on the BelongsTo class, is there any reason why it's not currently being used when adding constraints?

@taylorotwell taylorotwell merged commit cff895d into laravel:11.x Nov 18, 2024
33 checks passed
@@ -378,8 +379,9 @@ public function getRelation($parent = null, $builder = null)
$this->builder = $builder ?: m::mock(Builder::class);
$this->builder->shouldReceive('where')->with('relation.id', '=', 'foreign.value');
$this->related = m::mock(Model::class);
$this->related->shouldReceive('getKeyName')->andReturn('id');
$this->related->shouldReceive('getcolumn')->andReturn('id');

Choose a reason for hiding this comment

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

Is this supposed to be getColumn?

Copy link
Member

Choose a reason for hiding this comment

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

doesn't really matter. method names are not case sensitive.

@crynobone
Copy link
Member

This PR broken morphTo() usage. If for example we explicitly add qualifyColumn(string $column) multiple tests will be broken indicating an issue.

crynobone added a commit that referenced this pull request Nov 19, 2024
taylorotwell pushed a commit that referenced this pull request Nov 19, 2024
…53568)

* wip

Signed-off-by: Mior Muhammad Zaki <[email protected]>

* wip

Signed-off-by: Mior Muhammad Zaki <[email protected]>

* Revert "fix: use qualifyColumn rather than assuming format (#53559)"

This reverts commit cff895d.

* wip

Signed-off-by: Mior Muhammad Zaki <[email protected]>

* wip

Signed-off-by: Mior Muhammad Zaki <[email protected]>

* wip

Signed-off-by: Mior Muhammad Zaki <[email protected]>

---------

Signed-off-by: Mior Muhammad Zaki <[email protected]>
@crynobone
Copy link
Member

This PR has been reverted due as it triggers error on certain MorphTo relationships. See #53568

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.

5 participants