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

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

Closed
helderneves91 opened this issue Oct 25, 2023 · 23 comments · May be fixed by #2548
Closed

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

helderneves91 opened this issue Oct 25, 2023 · 23 comments · May be fixed by #2548

Comments

@helderneves91
Copy link

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
Copy link
Author

helderneves91 commented Oct 25, 2023

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

Let me say it again, the role exists.

@helderneves91
Copy link
Author

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
Copy link
Contributor

erikn69 commented Oct 25, 2023

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
Copy link
Collaborator

drbyte commented Oct 30, 2023

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

@erikn69
Copy link
Contributor

erikn69 commented Oct 30, 2023

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.

@drbyte drbyte closed this as completed Oct 31, 2023
@mohammedALhakmy
Copy link

Don't sea the answer my question

@erikn69
Copy link
Contributor

erikn69 commented Nov 6, 2023

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

@mohammedALhakmy
Copy link

There is no permission named 1 for guard web.

this is error

@mohammedALhakmy
Copy link

mohammedALhakmy commented Nov 7, 2023 via email

@drbyte
Copy link
Collaborator

drbyte commented Nov 7, 2023

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
Copy link
Collaborator

drbyte commented Nov 7, 2023

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
Copy link
Collaborator

drbyte commented Nov 7, 2023

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
Copy link

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
Copy link
Collaborator

drbyte commented Nov 7, 2023

$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
Copy link

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

@drbyte
Copy link
Collaborator

drbyte commented Nov 7, 2023

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

@mohammedALhakmy
Copy link

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
Copy link
Collaborator

drbyte commented Nov 7, 2023

$role->syncPermissions();

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

@mohammedALhakmy
Copy link

what is the correct solution and code

@drbyte
Copy link
Collaborator

drbyte commented Nov 7, 2023

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)));

@mohammedALhakmy
Copy link

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

@webdevmahabub
Copy link

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
Copy link

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!!

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 a pull request may close this issue.

6 participants