use table name for belongsToMany()

[Jehong-Ahn]

belongsToMany() needs a table name always.

if ($is_pivot) {
    ...
    $relationship = sprintf('$this->%s(%s::class, \'%s\')', $type, $fqcn, $foreign->tableName());
    ...
}
else {
    $relationship = sprintf('$this->%s(%s::class, \'%s\')', $type, $fqcn, $column_name);
}

[jasonmccreary]

I don't think it's true that it always needs a name. Only when it's unconventional. In addition, all tests are failing.

[Jehong-Ahn]

I know table name convention. But, how can I use unconventional pivot table name?

I didn't test the whole case. My mistake. Here is the missing commit. https://github.com/laravel-shift/blueprint/compare/master...Jehong-Ahn:blueprint:belongsToMany-tablename

But why close so quickly? Always do you?

[ghostwriter]

Hey @Jehong-Ahn,

Please don't take offense, it's not personal.

Closing issues/pull requests early is normal practice that helps maintainers focus on bugs and features that require attention by only leaving those issues open.

Plus we can always reopen any issue/pull request for further discussion.

Now on to reviewing this patch.

---

I've made the same mistake before, don't change existing test fixtures unless you are fixing a bug.

Instead, create a new draft file in the fixtures directory.

---

I don't think this pull request is necessary because, correct me if I'm wrong, Laravel automatically uses the value of $table on the Pivot model to determine the table name which can be set via meta configuration.

Membership:
    meta:
      pivot: true
      table:custom_table_name
    user_id: id
    team_id: id

class Membership extends Pivot
{
    protected $table = 'custom_table_name';
}

class User extends Model
{
    public function teams(): BelongsToMany
    {
        return $this->belongsToMany(Team::class) // <-- no need to add Pivot table name
            ->using(Membership::class) // <- because table name is derived from `Membership::$table`
            ->as('membership')
            ->withPivot('id')
            ->withTimestamps();
    }
}

class Team extends Model
{
    public function users(): BelongsToMany
    {
        return $this->belongsToMany(User::class) // <-- no need to add Pivot table name
            ->using(Membership::class) // <- because table name is derived from `Membership::$table`
            ->as('membership')
            ->withPivot('id')
            ->withTimestamps();
    }
}

Thanks again for your contributions.

[Jehong-Ahn]

Thanks, @ghostwriter

Unfortunately, laravel doesn't use table name property for pivot relation. On belongsToMany(), laravel forces conventional table name.

Maybe this issue is on the laravel, not blueprint.

https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Eloquent/Concerns/HasRelationships.php#L513

        // If no table name was provided, we can guess it by concatenating the two
        // models using underscores in alphabetical order. The two model names
        // are transformed to snake case from their default CamelCase also.
        if (is_null($table)) {
            $table = $this->joiningTable($related, $instance);
        }

[Jehong-Ahn]

https://github.com/laravel/framework/issues/29571

[ghostwriter]

No, it's not on Laravel's side.

You're right, I believe I understand what you're looking for.

It's seems to me like we need to implement something that tells blueprint to use custom table name that's passed to meta.table configuration.

Only If

1. meta.table was set to a non-empty-string in draft file
2. meta.table is !== the convention snake cased models sorted alphabetically and concatenated with an underscore
3. meta.pivot is set to true.

Adding a test that explicitly tests these requirement should get the rest of the tests passing and achieve what you need.

[ghostwriter]

My previous answer is based on the recommendations https://github.com/laravel/framework/issues/29571#issuecomment-521333335

It's definitely worth discussing with Laravel internals to see if they will accept a pull request to make sure Laravel starts using the $table property on the pivot models.

[jasonmccreary]

PRs that don't have the correct change, tests, or a feature I wish to support within Blueprint get closed. That's how it works. If you want, you are welcome to fork this package and maintain your own version. 👍

If there is something you feel should still be added, please leave a clear comment demonstrating the problem.

[Jehong-Ahn]

OK. Thanks for commenting. 👍

[dansleboby]

Thanks, @ghostwriter

Unfortunately, laravel doesn't use table name property for pivot relation. On belongsToMany(), laravel forces conventional table name.

Maybe this issue is on the laravel, not blueprint.

https://github.com/laravel/framework/blob/10.x/src/Illuminate/Database/Eloquent/Concerns/HasRelationships.php#L513

        // If no table name was provided, we can guess it by concatenating the two
        // models using underscores in alphabetical order. The two model names
        // are transformed to snake case from their default CamelCase also.
        if (is_null($table)) {
            $table = $this->joiningTable($related, $instance);
        }

Hi if you use something like this it will take the table name in the pivot class

$this->belongsToMany(Team::class, Membership::class) // <-- no need to add Pivot table name
            ->using(Membership::class)

[Jehong-Ahn]

Thank you @dansleboby