laravel-permission: hasPermissionTo never checks roles (because HasRoles trait does not work with Collection)

“can’t understand … doesn’t mean they’re badly done”

Human-readability is important, you’re basically saying:

• readers are forced to understand,
• without any instanceof check,
• and even without any comment.

But there are multiple other things badly done in that line, beside not being human-readable:

• if we pass to that method a custom class,
• which has a custom intersect method,
• said method is called, and maybe drops all tables, or something like that.
• it’s a custom method, and it’s allowed to do whatever it wants.
• it’s your fault for calling that method without checking 😉

---

“… it is very tested (11k stars, sure you didn’t give one)”

• I gave one star 😉
• Starts don’t prove something is 100% well done,
• They’re badly done wherever they’re not well done.

It simply uses $this->roles, even if “roles” relation is never loaded. roles are always loaded with $this->loadMissing(‘roles’);

Hello, if you think there is a bug, do a failing test to prove it, also feel free to make a PR to make the code better from my point of view the problem is that you can’t understand the code(apparently you are the only one), because there are many tests on this package https://github.com/spatie/laravel-permission/tree/main/tests

@drbyte @freekmurze spam here https://stackoverflow.com/users/8740349/top-master

Here:

the old saying of “we accept pull requests”, which’s basically just dodging work.

This is open source, it is not a paid job where you order something and everyone must put to work to please you, I am not the author nor a member of the staff, I only review issues in my spare time to help, and this is not an bug

Open source software is developed in a decentralized and collaborative way, relying on peer review and community production.

That means that if you can improve something you can collaborate instead of complaining and belittling the work of others

Wrong, re-read https://github.com/spatie/laravel-permission/issues/2332#issuecomment-1436704055

Ok

readers are forced to understand,

Not, easy to understantd to me, do not generalize with your personal appreciations, but of course depending on the level of knowledge or intellectual capacity something is difficult or easy to understand, when I read a laravel package for the first time it seemed difficult to understand, now I understand it with a single glance

without any instanceof check,

If all supported checks passed, the main one supported remains by elimination(I didn’t write that code but I got it the first time), It’s called early returns, read about it, also if you check instanceof Collection, still might not be a collection of roles, human error can be more extensive and you won’t finish the checks, you will only increase the cost of performance

and even without any comment.

no need to comment on the obvious things, but i gonna point you the comment with the supported datatypes https://github.com/spatie/laravel-permission/blob/490fa00b0c8286659a534d822f7e74c684c86d27/src/Traits/HasRoles.php#L186-L190

if we pass to that method a custom class it’s a custom method, and it’s allowed to do whatever it wants.

I’m using custom classes and everything works, but when using custom classes you must prevent any incompatibility by your own responsibility, it is something that everyone knows, no package can predict all possible customizations, extendings, and others

which has a custom intersect method,

intersect comes from laravel collection, I doubt someone will change that utility but if it does, read the previous point again, but when they drop PHP 7 support for sure they will do typed methods, for now it seems that no one has come up with the idea of ​​passing unsupported datatypes(yes, it could be better but if the community needs that change, a PR has already been done)

said method is called, and maybe drops all tables, or something like that

lol, in 15 years I have never seen anything like this happen in any framework, If someone is capable of doing that, it is because they are doing their job very badly, normally when you do some customization you also do unit tests to avoid unexpected behaviors

it’s your fault for calling that method without checking

Tested, it’s not a bug, if you think there is a bug, do a failing test and surely someone would take you seriously

don’t try to change the subject.

You can’t order that, authors can modify what they want in their repository without your permission or approval.

Well, that line does not even exist, for example, on version 3 and 4 branches, I will try updating to version 5 (current latest) once possible 😉

This information is very important before opening issues, everyone’s time is valuable

well, I think I have answered all the questions, bye

Finally there it is, the old saying of “we accept pull requests”, which’s basically just dodging work.

Also, you seem to point again and again to:

$this->loadMissing('roles');

Well, that line does not even exist, for example, on version 3 and 4 branches, I will try updating to version 5 (current latest) once possible 😉

However, that is history by now, what about the second issue, don’t try to change the subject.

---

“you can’t understand”

Wrong, re-read my comment, again and again until you understand, lol.

How much is even your StackOverflow reputation? I mean, please don’t spam this thread until you have at least same: 7000

I openned issue without reading that last line, which you call “Collection handler”.

Basically, instead of above issue, I should open 2 new issues.

First issue

It simply uses $this->roles, even if “roles” relation is never loaded.

Instead, it should throw an exception, like:

“roles relation not loaded”

Basically, just returning false is a lie in this case.

Second issue

It does ->intersect without checking if $roles is instanceof Collection.

Which basically was hidding it from my review, I mean, you’re lucky I don’t review your code, I would reject that instantly, because instanceof Collection check is missing.

remember the good old “Silence is golden.”

@GitHub

Check these contributors’ accounts, we see almost zero commitment to anything, but yet they are pretty active on spatie packages only.

Maybe those are fake accounts created by members, which allows them to comment whatever they want, without damaging their reputation. Also, the number of contributors which GitHub shows increases, which makes the package look more active.

Unfortunately, seems GitHub has not any check present to prevent said matter, and even reporting them didn’t do anything.

keep the package as simple as it can be Freek Van der Herten

No one has spammed you or given you excuses, most of the responses you received from sporadic collaborators are correct

They’re badly done

This is software from Spatie, reviewed by Freek Van der Herten, whose reputation is comparable to Taylor’s. He has created many plugins for Laravel and is one of his regular collaborators. Any “bad code” complaints with the actual author, not the contributors

This is a belgium package for everyone, not all speak english, it is used as a standard to facilitate communication.

finally there it is, the old saying of “we accept pull requests”, which’s basically just dodging work.

Everyone is invited to collaborate, the nationality or reputation of any page is not taken into account for the reviews, answers, or issues reports

no one is “dodging work”, if that was the case you wouldn’t have even gotten all your questions answered

To avoid incorrect answers or confusion, specify all the information required in ISSUE_TEMPLATE/bug_report.md

Have a nice day

@top-master if you don’t like this package, there is other alternatives, no one forces you to use this package, and yes, grow up, both https://github.com/spatie/laravel-permission/blob/490fa00b0c8286659a534d822f7e74c684c86d27/README.md?plain=1#L92-L97

“only complain about readability when it suits”

That image is not code, but if it was, it would contain instanceof check, unlike your gibberish, lol.

---

Yes, that’s called edit, go complain to GitHub if you want the old version.

Also, look who is talking:

Good joke, or good spam.

No one has spammed you, check the meaning of spam, so far everyone has answered your questions, despite your behavior

"Use markdown correctly,```php

Apparently you don’t know how markdown works, I’ll show you if you use it correctly you can use hightlight, and that does help the readability

```php

array(
   'test' => 'test'
)

```js

{
   test : "test"
}

here not readability

at least Laravel does nothing laravel just create one empty db row without any exception

Lol, you didn’t read, laravel creates an invalid row

Go report that to Laravel, others making same mistake is no excuse

Exactly, others making mistakes is no excuse, you mistakes are not bugs

“stackoverflow mostly it is to answer basic questions of the noobs”

@parallels999 If you can’t even answer a noob’s question, what does that make you - right, even worst noob 😉

Secondly, there are many pro questions, go answer those.

“So much reputation … specify the package’s version”

First of all, this thread is about current-latest version, hence no need to specify version, I mean, the version is only related to the first of the issues which I said I should open, all other issues are about current-latest version.

Secondly, I did not even open such issue(s) yet, hence you wasted your and my time by answering here instead of waiting for said issues to be openned.

Lastly, learn english.

---

@PaolaRuby I hope no one wastes time reading a spammer’s excuses, most of which did not make sense.

Of course, I meant inline-comment, the doc-comment does not explain why instanceof check is missing (and should never, we have inline-comment for that).

Well, that line does not even exist, for example, on version 3 and 4 branches, I will try updating to version 5 (current latest) once possible 😉

So much reputation and it seems that you have to explain the basics, first when you start a new issue report you must specify the package’s version, laravel’s version and PHP version, otherwise everyone will assume that you are in the current versions, For example, several here have reviewed what you say, and apparently you use an outdated version of the package and everything you report has probably already been fixed according to the tests, you only wasted the time of people with the intention of helping

Second, old versions reaches they end of life and they are without support, current is 5, and in a short time they will go to 6

You also arrive with your attitude that “everything is badly done and full of bugs” (without showing any failing tests), and “that I have a super reputation and I’m better than everyone”

stackoverflow reputation don’t prove something here, better show how many PHP contributions you have made to open source in this month at least, stackoverflow mostly it is to answer basic questions of the noobs

please don’t spam this thread until the day you beat my reputation

😄😄😄😄😄 This is a public thread, It is also necessary to explain how this works

It simply uses $this->roles, even if “roles” relation is never loaded.

roles are always loaded with $this->loadMissing('roles');

https://github.com/spatie/laravel-permission/blob/0c497767227d19eff395789e65262fb95068aec3/src/Traits/HasRoles.php#L190-L192

Which basically was hidding it from my review, I mean, you’re lucky I don’t review your code, I would reject that instantly, because instanceof Collection check is missing.

😄😄😄😄 Just because you can’t understand how things work doesn’t mean they’re badly done. As you say(before you deleted), this package is so many years old, it is because it is very tested(11k stars, sure you didn’t give one) Also, why do you delete your comments and their changes history, shame on nonsense?