laratrust
laratrust copied to clipboard
santigarcor
Add once() Helper Function To Role and Permission Checking.
This PR adds the new once() helper function to Laratrust along with the option to enable/disable it via the configuration file.
This is useful as it removes the need to override the hasRole() and hasPermission() methods on the user model and instead moves this logic into Laratrust itself.
The default is that the use of this function is disabled due to the once() function not being added into Laravel Core until Laravel 11. The inline documentation comment with this configuration states that the application must be running Laravel 11 in order to use this function. The comment also states that if the application is running Laravel 10 or below, they need to run composer require spatie/once
I like the idea, could you please add the tests for it?
Also, I have a question, does the once method takes into consideration if the callback is using different params?
@santigarcor I can try to add these tonight or tomorrow. My understanding is that if different values are passed to once() then the result will be different from the last time once() was called, But if you pass call once(1, 2, 3) multiple times, the DB lookup will only occur once.
Please let me know if that answers your question or if I can explain it better.
@santigarcor I just added the configs into the already existing UserTest.php. From what I can see, this should test the expected behavior. Please let me know if I'm misunderstanding anything.
No, those lines you added are not testing anything there. You should place them here. that's the base class for all checks.
@santigarcor I believe I added the tests to the proper file. Can you please verify that this is what you wanted me to do? If there's anything else, please let me know.
@santigarcor looking at the tests, I see that only the Laravel 10 tests are failing, which I would expect since once is not included in Laravel 10 and this package doesn't require spatie/once.
I'm not sure what the best method would be for getting the Laravel 10 tests to pass. Is there a way to detect the version of Laravel being run in the tests? The only thing that comes to mind is the Composer Runtime API.
Thoughts?
I think we need to find another solution for this, because if we release this for version 8.x it has support for 10.x and 11.x. I won't do it for version 9.x because i'm thinking on a rewrite.
@santigarcor So my brain completely forgot about the function_exists() PHP function. I added this to the Config::get('laratrust.cache.once') check. So now the developer will need to enable laratrust.cache.once and the once() function will need to be registered.
once() registration will occur by installing the spatie/once package in L10 and automatically in L11. As far as the tests are concerned, once() does not exist in the L10 tests, so the current behavior will not be altered to use once() even if laratrust.cache.once is set to true