Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Role::getTable() method brokes withCount method #2277

Closed
xenaio-daniil opened this issue Dec 18, 2022 · 1 comment · Fixed by #2280
Closed

Role::getTable() method brokes withCount method #2277

xenaio-daniil opened this issue Dec 18, 2022 · 1 comment · Fixed by #2280

Comments

@xenaio-daniil
Copy link
Contributor

xenaio-daniil commented Dec 18, 2022

I needed to get ManyToMany relation of roles (structure of departments). To do this, I extended the \Spatie\Permission\Models\Role class with methods:

class Department extends \Spatie\Permission\Models\Role
{
    ....
    public function parents(): BelongsToMany
    {
        return $this->belongsToMany(
            Department::class,
            "department_hierarchy,
            'child_id',
            'parent_id'
        );
    }

    public function children(): BelongsToMany
    {
        return $this->belongsToMany(
            Department::class,
            'department_hierarchy',
            'parent_id',
            'child_id'
        );
    }
    ....
}

and created migration:

    Schema::create('department_hierarchy', function (Blueprint $table) {
        $table->string("rel_id", 25)->primary();
        $table->bigInteger("parent_id", false, true);
        $table->bigInteger("child_id", false, true);
        $table->boolean("is_direct")->default(false);
        $table->foreign("parent_id")->references("id")->on("roles");
        $table->foreign("child_id")->references("id")->on("roles");
    });

Everything works well except methods like

\App\Models\Department::withCount('children')

and so on: it always returns parent_count = 0.

The reason of this behavior is in overriding of method getTable in Spatie\Permission\Models\Role.

When extending Illuminate\Database\Eloquent\Model:

echo \App\Models\Department::withCount('children')->toSql();
/**
SELECT `roles`.*, (
SELECT COUNT(*)
FROM `roles` AS `laravel_reserved_0`
INNER JOIN `department_hierarchy` 
    ON `laravel_reserved_0`.`id` = `department_hierarchy`.`child_id`    <-- look at laravel_reserved_0
WHERE `roles`.`id` = `department_hierarchy`.`parent_id`) AS `children_count`
FROM `roles`

When extending \Spatie\Permission\Models\Role:

echo \App\Models\Department::withCount('children')->toSql();
/**
SELECT `roles`.*, (
SELECT COUNT(*)
FROM `roles` AS `laravel_reserved_0`
INNER JOIN `department_hierarchy` 
    ON `roles`.`id` = `department_hierarchy`.`child_id`    <------ laravel_reserved_0 != roles
WHERE `roles`.`id` = `department_hierarchy`.`parent_id`) AS `children_count`
FROM `roles`

withCount() uses Model::table field to temporary store alias name (laravel_reserved_0).

And when (in Illuminate\Database\Eloquent\Model) method Model::getTable() returns $this->table or magic name from class basename, your version returns only pre-configurated table name (config('permission.table_names.roles')) or, if configuration not found, Model::getTable(). So if permission.table_names.roles is configured, it has no change to return $this->table.

I would recommend to rewrite this method like this:

    public function getTable()
    {
        return $this->table ?? config('permission.table_names.roles', parent::getTable());
    }

or for best practice remove method Role::getTable() and replace it with

    public function __construct(array $attributes = [])
    {
        $attributes['guard_name'] = $attributes['guard_name'] ?? config('auth.defaults.guard');

        parent::__construct($attributes);

        $this->guarded[] = $this->primaryKey;

        $this->table = config('permission.table_names.roles', parent::getTable()); // <---------------
    }
@parallels999
Copy link
Contributor

Feel free to open a PR

xenaio-daniil added a commit to xenaio-daniil/laravel-permission that referenced this issue Dec 20, 2022
drbyte pushed a commit that referenced this issue Feb 8, 2023
* Fix Role::withCount if belongsToMany declared

fixes #2277
wakeup0706 added a commit to wakeup0706/laravel-permission that referenced this issue Feb 13, 2025
* Fix Role::withCount if belongsToMany declared

fixes spatie/laravel-permission#2277
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
2 participants