search

JosephSilber / bouncer

Laravel Eloquent roles and abilities.
MIT License
3.45k stars 330 forks source link

Remove null checks and allow it to be queried #608

Closed grantholle closed 1 year ago

grantholle commented 2 years ago

Fix #607. This has the expected behavior and not so eager to grant permissions when you may not want them to be granted.

grantholle commented 1 year ago

Also, if you don't mind adding the hacktoberfest-accepted tag to this, it would be really helpful in my goal of contributing during Hacktoberfest. Thanks!

grantholle commented 1 year ago

I've ran into the same situation when checking roles assignment. I have a scoped assigned role, but when I check $user->isA($role) when there isn't a scope set, it still passes.

I will try to include an update for that.

grantholle commented 1 year ago

Hi @JosephSilber, any plans to get this merged? Thanks for taking the time!

JosephSilber commented 1 year ago

Whoops. I should've gotten to this way sooner, especially since you put in all that work creating the issue, the repro repo and a PR with tests! Sorry.

I left you a small review. Check it out, and let's get this merged.

grantholle commented 1 year ago

Thanks man, I've made those suggested edits. No worries on the delay!

grantholle commented 1 year ago

Never mind, my local repo was out of date with this. Anyway, you were right on all of the suggestions.

JosephSilber commented 1 year ago

Merged. Thanks!

mikerockett commented 1 year ago

Well, this broke my app, but in a good way because it turns out we had scopes set when they weren't even being used (best guess is experimentation 😅).