-
-
Notifications
You must be signed in to change notification settings - Fork 381
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
Add use LaratrustUserTrait
with a command call + some fixes
#4
Conversation
Update DocBlocks.
in case user doesn't add an alias.
@KKSzymanowski i find this PR awesome, it will reduce the configuration complexity a lot. To be honest i'm no quite good at testing yet, but i made some digging and i think we could make something like this I'll be merging this PR to test it out and work on the Docs |
@KKSzymanowski you should add the code you are doing in the |
Yep, I don't know why I didn't. The idea started with |
@KKSzymanowski amazing work you did, congrats! I was just about to tell you that we shouldn't add the On another note, what terminal / command line are you using? Looks pretty neat (I'm a mac user but I'd like to know) |
@Mathius17 In the video? It's nothing special - it's the built in terminal in PhpStorm. Only thing I changed with it was the background color(to match the Tibau theme) and probably font size. Changed the default keybinging to quicky open it with F12 and it works great apart from cases when I do a lot of testing with phpunit, then scrolling up shows some bullshit(all that's needed to fix it is a terminal restart). About configuration - one last thought. I forgot completely about this but we should create |
@KKSzymanowski Yesterday i was reviewing the code, and i thought that, we should do it as the user trait, following the config |
Hi
Regarding issue #3:
New commands
laratrust:make-role
- Creates a Role model based on a stublaratrust:make-permission
- As above, creates a Permission modellaratrust:add-trait
- Adds aLaratrustUserTrat
use statement to User model in a manner described here (class name taken from config, so not necessarily \App\User)laratrust:setup
- Calls(in that order) following commands, informing before each step what is going to happen now.laratrust:migration
laratrust:make-role
laratrust:make-permission
laratrust:add-trait
Here's how it works, in HD of course:

Major fixes
\Laratrust
call from blade directive withapp('laratrust')
.The reason is, user isn't supposed to be obligated to add an
alias
so we cannot rely on it's existance. I know Zizaco implemented it with a analias
, but take a look for example at the @lang directive. It's exactly what I've done.Do you think we should block duplication, not just warn about it?
I think it can be more reliable by checking if
LaratrustSetupTables
class exists, but for now I think it's fine.Minor fixes
BladeCompiler
out of IoC container instead of accessing it via a Facade.I personally agree that using Facades(or whatever you might wanna call it) can implicate bad programming habbits(Taylor on Facades) and when they're not necessary, they should be replaced with Dependency Injection. That's one reason. Second reason why I did it this way is the sheer performance gain.
To be honest,
Traitor
is my first serious composer package and this is my first serious GitHub contribution so feel free to point out any mistakes or ways to improve this proposal.Traitor
is quite heavily tested so there souldn't be any problems on it's side. However I couldn't figure out how to test these commands. Do you have some ideas?