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.

Sign up for GitHub

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

[5.8] Allow belongsToMany to take Model/Pivot class name as a second parameter #27774

Merged
merged 4 commits into from
Mar 5, 2019
+34 −1
Merged

[5.8] Allow belongsToMany to take Model/Pivot class name as a second parameter #27774

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5518367
Added resolveTableName method to support direct pivot class assignment
linaspasv Mar 4, 2019
91fd13d
CS fix
linaspasv Mar 4, 2019
a8d04c5
Slightly adjusted code flow
linaspasv Mar 5, 2019
e1952ab
Tests added
linaspasv Mar 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class BelongsToMany extends Relation
public function __construct(Builder $query, Model $parent, $table, $foreignPivotKey,
$relatedPivotKey, $parentKey, $relatedKey, $relationName = null)
{
$this->table = $table;
$this->table = $this->resolveTableName($table);
$this->parentKey = $parentKey;
$this->relatedKey = $relatedKey;
$this->relationName = $relationName;
Expand Down Expand Up @@ -187,6 +187,31 @@ protected function performJoin($query = null)
return $this;
}

/**
* Resolves table name from a given string.
*
* @param string $class
* @return string
*/
protected function resolveTableName($class)
{
if (! class_exists($class)) {
return $class;
}

$object = new $class;

if ($object instanceof Model) {
if ($object instanceof Pivot) {
$this->using($class);
}

return $object->getTable();
}

return $class;
Copy link
Contributor

@staudenmeir staudenmeir Mar 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

What's a scenario where this line (212) is reached?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Line (212) is reached when a given table name can't be resolved as an instance of a Model class, so in all other cases it just fallback to the old behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

So for the rare case that a table name coincidentally is also a valid class name?

Copy link
Contributor Author

@linaspasv linaspasv Mar 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

My code checks if the given table name is a valid class name but also extends Model / Pivot class. The only rare case I could think of could be when user has a Pivot class named "customers_profiles " without a namespace and uses "customers_profiles" string as a second parameter for belongsToMany:

// note: no namespace used

class customers_profiles extends Pivot
{
    protected $table = 'customers_profiles';
}

......

return $this->belongsToMany(Profile::class, 'customers_profiles')

In this case, my code will see that's the valid Pivot class, will try to load Pivot model and take table name through getTable() method.

dd((new customers_profiles)->getTable()); 
// gives back "customers_profiles" string, 
// or 
// auto-generated value "customers_profile" when user has not defined $table attribute

All other cases I could think of should fallback to an old behavior.

I think the following very rare case should not be an issue. Let me know if I missed something.

Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

p.s. I have slightly adjusted the code flow / naming.

}

/**
* Set the where clause for the relation query.
*
Expand Down