Update not_for_release.php
Stop using @ and actually check if the directory exists.
Prior to PHP 8.0.0, the error_reporting() called inside the custom error handler always returned 0 if the error was suppressed by the @ operator. As of PHP 8.0.0, it returns the value E_ERROR | E_CORE_ERROR | E_COMPILE_ERROR | E_USER_ERROR | E_RECOVERABLE_ERROR | E_PARSE.
Merge pull request #5665 from brittainmark/patch-1
Update not_for_release.php
Stop using @ and actually check if the directory exists.
Prior to PHP 8.0.0, the error_reporting() called inside the custom error handler always returned 0 if the error was suppressed by the @ operator. As of PHP 8.0.0, it returns the value E_ERROR | E_CORE_ERROR | E_COMPILE_ERROR | E_USER_ERROR | E_RECOVERABLE_ERROR | E_PARSE.
Hence, errors are being produced.
This PR is for testing discussion #2367, this could be a breaking change if someone uses getPermissionClass(), getRoleClass()
Tests with this change
-> Time: 00:09.050, Memory: 46.00 MB
Test without this change
-> Time: 00:09.427, Memory: 48.00 MB
Seems like this change uses less memory, also I think that this uses less cycles because it is not calling app()
helper on every instance, and avoid making contract instances on every time(Tested on PHP 8.1)
Return string on getPermissionClass(), getRoleClass()
Merge pull request #2368 from erikn69/patch-8
[V6] [BC] Return string on getPermissionClass(), getRoleClass()
we have to add a note to the upgrade guide
Agreed.
Remove unnecessary qualifiers & Add some typed properties & Add some return types
Thank you for your contribution; your time is appreciated. However we'll not be merging it at this time.
Thank you for your contribution; your time is appreciated. However we'll not be merging it at this time.
Describe the bug I have an application where I have regular users with no roles and admins and employees with corresponding roles. I am wrapping admin-area in the role middleware, which works well. However, when I ask for a resource in that area, that returns 404 for the logged in admin and employee users, it also returns 404 for the logged in normal user. Would expect it to return 403, to not give away any details about what exists and what doesn't.
Versions "spatie/laravel-permission": "^5.9", "laravel/framework": "^10.0",
PHP version: 8.2
To Reproduce Steps to reproduce the behavior: Wrap a resource controller in role middleware. Access an unknown entity
Expected behavior Expecting HTTP 403 instead of 404
Additional context I use the automatic model injection from routes to controllers. I suspect this is the culprit in this.
Use anonymous migrations
Merge pull request #2374 from erikn69/patch-6
[V6] Use anonymous migrations
Since Laravel 8.x it is possible to use anonymous migrations, I don't think anyone starts a new project using Laravel <=7.x It's time to change migrations
@erikn69 I'm good with this, unless you feel there's more to explore with it before merging.
Do you want to rebase it onto latest main
(where test suite was refactored a bit) and update the tests' namespace from Test
to Tests
, before merging?
(Or I can do that after merging, either is fine.)
Refactor Tests
Test
to Tests
Tests\TestModels
Thanks to @AyoobMH - ref PR #2240
Co-authored-by: AyoobMH ayoob.mohammad98@gmail.com
Fix styling
Merge pull request #2375 from spatie/refactor-tests
Refactor Tests
Test
to Tests
Tests\TestModels
Thanks to @AyoobMH - ref PR #2240
Test
to Tests
Tests\TestModels
Thanks to @AyoobMH - ref PR #2240
@lk77 will this PR cause problems to your custom models?
Merged to v6-dev. Should we backport this to 5.x as well?
Fix nesting tests
Merge pull request #2364 from erikn69/patch-6
Fix nesting tests
In #2280 a new Migration, and Custom Model were added only for one test, also a complex process for changing migration was added, this PR deletes those files and use existing ones, also i did try to make test more simple
Also in #2280 the consistency between the Permissions and Roles models was lost, i did delete function getTable()
on permission to match roles model
Change clearClassPermissions to clearPermissionsCollection
Merge pull request #2369 from erikn69/patch-9
[V6] Rename clearClassPermissions method to clearPermissionsCollection
Discussed on #2367 use a more descriptive name
Thanks for this. The descriptive name is helpful.
FYI, I think there may be an English-language anomaly that's caused some confusion. Using "clearClassPermissions" and "* Clear class permissions" as a name/description, the implication is to "clear the permissions that the class holds", or "clear the class's permissions", where "class" is referring to the registrar. However, you are correct that "clearPermissionsCollection" is more descriptive and readable, and potentially less confusing.