rappasoft / laravel-boilerplate

The Laravel Boilerplate Project - https://laravel-boilerplate.com
https://rappasoft.com
5.59k stars 1.58k forks source link

Will the be updated to 5.1 #14

Closed Denoder closed 9 years ago

Denoder commented 9 years ago

I see a bunch of changes implemented especially dealing with user auth and something affecting the blade compiler.

freddyheppell commented 9 years ago

I imagine it'll be updated to every major laravel version

rappasoft commented 9 years ago

Yeah it will be updated providing there will be an upgrade guide which I'm sure there will. I'm going to refresh and start from scratch with every major release.

Anthony Rappa

On May 16, 2015, at 13:38, Freddy Heppell notifications@github.com wrote:

I imagine it'll be updated to every major laravel version

— Reply to this email directly or view it on GitHub.

jekinney commented 9 years ago

With the update to middleware with injecting dynamic parameters, and a acl that is provided, it is almost a must :)

rappasoft commented 9 years ago

Haha yes I haven't looked into all of the new features yet. But I'm sure they will all come in handy. I want it to be a boilerplate as well as a good learning tool for all the main components.

Anthony Rappa

On May 16, 2015, at 14:34, jekinney notifications@github.com wrote:

With the update to middleware with injecting dynamic parameters, and a acl that is provided, it is almost a must :)

— Reply to this email directly or view it on GitHub.

Denoder commented 9 years ago

__> - I decided to upgrade it myself :P - no problems so far.

rappasoft commented 9 years ago

Which guide did you use?

Anthony Rappa

On May 16, 2015, at 14:52, Christian notifications@github.com wrote:

__> - I decided to upgrade it myself :P - no problems so far.

— Reply to this email directly or view it on GitHub.

Denoder commented 9 years ago

i just used: http://laravel.com/docs/master/upgrade

then i found issues with the blade extension since it removed "createPlainMatcher" in the blade compilers for you know.... reasons :/ - so now the matcher is:

$matcher = '/@endpermission/'; $matcher = '/@endrole/';

Denoder commented 9 years ago

Also the Middleware now uses parameters so you could do something like this:

public function handle($request, Closure $next, $permissions = null)

rappasoft commented 9 years ago

Oh awesome I can get rid of some classes and make it simple now.

Anthony Rappa

On May 16, 2015, at 15:25, Christian notifications@github.com wrote:

Also the Middleware now uses parameters so you could do something like this:

public function handle($request, Closure $next, $permissions = null)

— Reply to this email directly or view it on GitHub.

rappasoft commented 9 years ago

Do you know if theres updated docs for the features yet? I don't see them, I just want to see how the route params are implemented and such. I will push a feature branch tomorrow for you to test, but it seems the only thing I had to change was bootstrap.php and create a cache directory, nothing else should be effected, I think. I'm gonna try to get rid of the Registrar file too my moving the methods somewhere else, suggestions are welcome.

Denoder commented 9 years ago

Middleware parameters: https://laracasts.com/series/intermediate-laravel/episodes/8

not much has been re-done for laravel 5.1 but ill find some more things to check out.

Denoder commented 9 years ago

also take a look at this compiler http://laravel.com/api/5.0/Illuminate/View/Compilers/BladeCompiler.html

vs this compiler (5.1) http://laravel.com/api/master/Illuminate/View/Compilers/BladeCompiler.html

as you can see they removed: createPlainMatcher <- used in your Blade Access Extender createOpenMatcher createMatcher


Take a look at your user roles and permissions , try adding a permission, then removing one. they changed this lists() method into a collection so to have they as an array, you would use lists()->all()

rappasoft commented 9 years ago

Ok i'll take a look. The bad thing is I'm not that good at writing tests, so there are none right now, I have to attempt to check each method call to make sure they work. I think I got the blade functions working from your previous comment, I think that was all that had to be done.

rappasoft commented 9 years ago

I'm not sure if I like the route middleware params, it doesn't seem flexible. For example, how would I pass an array as a parameter?

Edit: They're saying you can't, I could pass a JSON string and decode it in the middleware file, but that seems more complicated than it's worth.

Denoder commented 9 years ago

i am also not good at writing tests, so what i usually do is test all functions like a normal consumer and see if something breaks, when it does, I can find where it is located... that's how it works for me.

Affected files that need to be changed to lists()->all() App\Http\Controllers\Backend\Access\PermissionController.php App\Http\Controllers\Backend\Access\RoleController.php App\Http\Controllers\Backend\Access\UserController.php

so they would end with: lists('id')->all()


yea for the middleware u can't pass it as an array, they added it but it was made into a simple function, im trying to see if they can do something about that, cause that's limiting on many factors.

I prefer to use my middleware in the controllers cause then things look a lot more clumpy in the routes.

rappasoft commented 9 years ago

Maybe, I prefer it in the routes so I can see it all at a glance, plus my middlewares don't work in the controllers. I don't know which way it should go, i'm not crazy about the parameters, as long as the way it works now is still going to work, why change it?

Denoder commented 9 years ago

the way it works now is fine. The only reason is that it "doesnt" work in the controllers. So i fixed it up myself to work in the controllers. It works in the controllers but at the same time now it doesn't work in the routes (efficiently)

The middleware is pretty one sided, I liked the before/after filters back in 4.x >_>.

rappasoft commented 9 years ago

How did you implement it? Maybe I can make it work both ways.

Denoder commented 9 years ago

not really much done:

    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Closure  $next
     * @return mixed
     */
    public function handle($request, Closure $next, $permission = null)
    {
        $assets = $this->getAssets($request);

        if (! access()->canMultiple($permission, $assets['needsAll']))
        {
            return $this->getRedirectMethodAndGo($request);
        }

        return $next($request);
    }

As i said it's pretty much one sided and will basically favor controllers....

This is basically the parameter within laravel:

    /**
     * Resolve the middleware name to a class name preserving passed parameters
     *
     * @param $name
     * @return string
     */
    public function resolveMiddlewareClassName($name)
    {
        $map = $this->middleware;

        list($name, $parameters) = array_pad(explode(':', $name, 2), 2, null);

        return array_get($map, $name, $name).($parameters ? ':'.$parameters : '');
    }

so in a controller it would be like this:

$this->middleware('access.routeNeedsPermission:testing_permission');
rappasoft commented 9 years ago

Why not do both? You can favor either or in the code.

public function handle($request, Closure $next, $permission = null)
    {
        $assets = $this->getAssets($request);

        if (! access()->canMultiple(! is_null($permission) ? $permission : $assets['permissions'], $assets['needsAll']))
        {
            return $this->getRedirectMethodAndGo($request);
        }

        return $next($request);
    }
rappasoft commented 9 years ago

Plus needsAll doesn't matter because you can't pass and array.

Edit: Or you can pass that as well. Permissions could be something like permission1|permission2, then do something like explode("|", $permission) to get the array of.

rappasoft commented 9 years ago

Did this quick, didn't test:

public function handle($request, Closure $next, $permissions = null, $needsAll = null)
    {
        $assets = $this->getAssets($request);
        $permissions = ! is_null($permissions) ? (strpos($permissions, "|") !== false ? explode("|", $permissions) : $permissions) : $assets['permissions'];
        $needsAll = ! is_null($needsAll) ? (bool)$needsAll : $assets['needsAll'];

        if (! access()->canMultiple($permissions, $needsAll))
            return $this->getRedirectMethodAndGo($request);

        return $next($request);
    }
Denoder commented 9 years ago

this works:

$this->middleware('access.routeNeedsPermission:can_manage_news|can_manage_titles');

for route parameters you would have something like this:

[middleware => 'access.routeNeedsPermission:can_manage_news|can_manage_titles']

so you would in turn remove the:

'permssions' => [],

and instead use the one above.

Denoder commented 9 years ago

the same would apply to the roles

Denoder commented 9 years ago

though the NeedsRoleOrPermssion file was already confusing for me so i didnt touch it >_>


i think it would be something like this:

    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Closure  $next
     * @return mixed
     */
    public function handle($request, Closure $next, $roles = null, $permissions = null, $needsAll = null)
    {

        $assets = $this->getAssets($request);
        $roles = ! is_null($roles) ? (strpos($roles, "|") !== false ? explode("|", $roles) : $roles) : $assets['roles'];
        $permissions = ! is_null($permissions) ? (strpos($permissions, "|") !== false ? explode("|", $permissions) : $permissions) : $assets['permissions'];
        $needsAll = ! is_null($needsAll) ? (bool)$needsAll : $assets['needsAll'];

        if ($assets['needsAll']) {

            if (! access()->hasRoles($roles, true) || ! access()->canMultiple($permissions, true))
                return $this->getRedirectMethodAndGo($request);

        } else {

            if (! access()->hasRoles($roles, false) && ! access()->canMultiple($permissions, false))
                return $this->getRedirectMethodAndGo($request);

        }
        return $next($request);
    }
rappasoft commented 9 years ago

What confuses you about it? I can probably simplify it, but it's a combination of the other two files in one.

Denoder commented 9 years ago

yea i noticed. I like to skim files so anything that seems really long i usually just don't like to deal with unless i need to.

(Not a fan of multiple lines of code thats if and else)

Denoder commented 9 years ago

Im glad L5.1 will have LTS because if i have to do another major upgrade to like L5.5 or L6 (or something of that variation) and the folder structure changes..... i will destroy all frameworks.

rappasoft commented 9 years ago

Did they change the folder structure this time? I didn't see it in the upgrade docs.

Denoder commented 9 years ago

na, i wa sreferring to the transition from L4 to L5. They did change the Commands folder to Jobs so now its:

Jobs/Jobs.php

rappasoft commented 9 years ago

I guess that wasnt a required change to upgrade then.

I might do it anyway.

On May 17, 2015, at 16:13, Christian notifications@github.com wrote:

na, i wa sreferring to the transition from L4 to L5. They did change the Commands folder to Jobs so now its:

Jobs/Jobs.php

— Reply to this email directly or view it on GitHub.

Denoder commented 9 years ago

it's not "required" they just did it to implicate that its not a command more of a job. so not changing it wont do anything.

jekinney commented 9 years ago

You can't pass an array per say into the middleware, but you can pass multiple parameters. Concatenate as many as you need separated by a colon (:) I believe.

jekinney commented 9 years ago

Taylor is also working on the docs now. Switch to master from 5.0 and that is the 5.1 docs in work.

rappasoft commented 9 years ago

Here's the branch: https://github.com/rappasoft/laravel-5-boilerplate/tree/feature/upgrade_to_5.1

I'm not done but you can pull it and see if you see anything wrong. Read the commit notes for what I added.

Denoder commented 9 years ago

works for me: also did throttling ever get implemented?

https://github.com/rappasoft/laravel-5-boilerplate/commit/7b42a2994f2c6e3f789698b9c005b09161a029f4

rappasoft commented 9 years ago

Not yet, have to research the best way to do it. I'll probably reverse engineer the zizaco/Confide version.

rappasoft commented 9 years ago

Also, working on way to use the access middleware fully in both routes and controllers.

Denoder commented 9 years ago

what do you mean by work both in routes and controllers. Dont they already work? I mean they work for me in the controllers.

rappasoft commented 9 years ago

All the parameters that come with the access library, for example:

Route::group([
            'middleware' => 'access.routeNeedsRole',
            'role'       => ['Administrator'],
            'redirect'   => '/',
            'with'       => ['flash_danger', 'You do not have access to do that.']
        ], function ()
        {
            Route::get('dashboard', ['as' => 'backend.dashboard', 'uses' => 'DashboardController@index']);
            require_once(__DIR__ . "/Routes/Backend/Access.php");
        });

Will be able to be:

$this->middleware('access.routeNeedsRole:{role:Administrator,redirect:/,with:flash_danger|You do not have access to do that.}');

Instead of having the route group.

Denoder commented 9 years ago

oh, the other stuff >_> - dint really like it. I was just mostly in it for the roles/permissions portion since it already redirects by itself.

rappasoft commented 9 years ago

It just gives more flexibility.

rappasoft commented 9 years ago

I have pushed all of the route/controller middleware updates: https://github.com/rappasoft/laravel-5-boilerplate/tree/feature/upgrade_to_5.1

rappasoft commented 9 years ago

Anything else you think I should change?

Denoder commented 9 years ago

not at the moment.

rappasoft commented 9 years ago

Cool, when it goes stable i'll merge it and give it a 1.0 tag finally.

hopewise commented 8 years ago

Hello, Is the Throttling feature ready to use in this boilerplate? I just read that such feature is built in with L5.1 https://mattstauffer.co/blog/login-throttling-in-laravel-5.1

hopewise commented 8 years ago

Any one?