spatie/laravel-permission

[v6] Possible bug on $user->syncRoles($array)

helderneves91 opened this issue · 23 comments

helderneves91 commented

Describe the bug
Hello, after upgrading to v6, I can't sync an array of role ids to an user.
As far I know, the v6 doesn't break the sync() method right? Am I missing something?

Versions
PHP 8.2
laravel , permissions, etc -- latest to date

Here is my example code and/or tests showing the problem in my app:
#2529

helderneves91 commented

I think the PR #2423 has something to do with this error.

Let me say it again, the role exists.

helderneves91 commented

Well, resolved the problem by using $user->roles()->sync($validated_data['roles']); discarding $user->syncRoles($validated_data['roles']);.

The same logic can be applied to $user->syncPermissions().

erikn69 commented

I think the PR #2423 has something to do with this error.

No, here #2089
I think the problem is your array,
the package needs an int array, like [42, 43, 44], but you have a string array, like ["42", "43", "44"], strings are taken as names, check it that

The functionality could be changed, is_int would have to be changed to is_numeric, but in that case number strings could no longer be used as permissions/roles names, I don't know if it would be a breaking change

drbyte commented

Is there something we should add to the upgrade docs to clarify this?

erikn69 commented

Is there something we should add to the upgrade docs to clarify this?

If my memory serves me right, this was always how it worked.

mohammedALhakmy commented

Don't sea the answer my question

erikn69 commented

@mohammedALhakmy what question?? I don't see any comments from you

mohammedALhakmy commented

There is no permission named 1 for guard web.

this is error

mohammedALhakmy commented
drbyte commented

It appears that your application is sending "1" as a string, instead of as an integer when querying a permission. Thus it is searching the name instead of the record ID.

Either pass an int for the permission record number, or a Permission Model instance, or a string of the permission name.

drbyte commented

It's always helpful when you provide actual code from your application. Without seeing your code it is much more difficult to give specific answers.

drbyte commented

i have this error with the in edit or store the users and roles why can

If you are assigning permissions based on end-user selection in a form, I am guessing that you are sending the permission ID to the form as a checkbox or dropdown, but when you receive the Request data you are not converting those Permission IDs back to int (all HTTP Requests from POSTs come as strings) before assigning the permission to the user.
If you're always using the record ID in the form, then always convert it back to int before using that ID in the assign/sync/remove/etc function calls.

mohammedALhakmy commented

this function with store roles and seam error with store ,edit users olsa store and edit rolse but this function store roles

public function store(RoleRequest $request){
    $check_Role = Role::where(['name'=>$request->name])->first();
    if (!empty($check_Role)){
        session()->flash('before_before');
        return  redirect()->route('roles.index');
    }else{
        $role = Role::create(['name' => $request->input('name')]);
        $role->syncPermissions($request->input('permission'));
        session()->flash('add_data');
        return redirect()->route('roles.users');
    }

}
drbyte commented

$role->syncPermissions($request->input('permission'));

If you dd($request->input('permission')) you will see that it is a string (numeric yes, but as a string, not an integer) or an array of strings.
Convert $request->input('permission') to int (or an array of int's) before calling syncPermissions().

mohammedALhakmy commented

this answer
dd($request->input('permission'))
array:3 [▼ // app\Http\Controllers\Admin\RoleController.php:50
0 => "1"
1 => "2"
2 => "3"
]

drbyte commented

Notice that "1" and "2" and "3" are strings.
Convert them to integers before passing them to sync

mohammedALhakmy commented

don't show error but not insert any check role just insert name role not check roles why
$check_Role = Role::where(['name'=>$request->name])->first();
if (!empty($check_Role)){
session()->flash('before_before');
return redirect()->route('roles.index');
}else{
$role = Role::create(['name' => $request->input('name')]);
$role->syncPermissions();
session()->flash('add_data');
return redirect()->route('roles.index');
}

drbyte commented

$role->syncPermissions();

Calling $role->syncPermissions() without any permission parameters will simply remove all permissions from the $role.

mohammedALhakmy commented

what is the correct solution and code

drbyte commented

Question: $request->input('permission') comes from your Form, right?
How does the 'permission' field get populated into your form? Where does that data come from?
Does it come from a database query, and you extract the numeric id's to put into the form?
And then the user ticks checkboxes to select some of those id's?

When those numeric id's come back into $request->input('permission'), they arrive as an array of strings.
Take that array of strings and convert it to integers. Perhaps with something like array_map:

$role->syncPermissions(array_map(fn($val)=>(int)$val, $request->input('permission')));

or

$role->syncPermissions(collect($request->input('permission')->map(fn($val)=>(int)$val)));
  • ❤️4
mohammedALhakmy commented

thank you so much,it is the correct is very good🤩🤩🤩🤩
$role->syncPermissions(collect($request->input('permission')->map(fn($val)=>(int)$val)));

webdevmahabub commented

Question: $request->input('permission') comes from your Form, right? How does the 'permission' field get populated into your form? Where does that data come from? Does it come from a database query, and you extract the numeric id's to put into the form? And then the user ticks checkboxes to select some of those id's?

When those numeric id's come back into $request->input('permission'), they arrive as an array of strings. Take that array of strings and convert it to integers. Perhaps with something like array_map:

$role->syncPermissions(array_map(fn($val)=>(int)$val, $request->input('permission')));

or

$role->syncPermissions(collect($request->input('permission')->map(fn($val)=>(int)$val)));

THANKS my issue solved : $permissionIds = collect($request->input('permission'))->map(fn($val) => (int)$val)->all();

tesseractT commented

Question: $request->input('permission') comes from your Form, right? How does the 'permission' field get populated into your form? Where does that data come from? Does it come from a database query, and you extract the numeric id's to put into the form? And then the user ticks checkboxes to select some of those id's?

When those numeric id's come back into $request->input('permission'), they arrive as an array of strings. Take that array of strings and convert it to integers. Perhaps with something like array_map:

$role->syncPermissions(array_map(fn($val)=>(int)$val, $request->input('permission')));

or

$role->syncPermissions(collect($request->input('permission')->map(fn($val)=>(int)$val)));

thank you this worked for me!!