Skip to content
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

[5.6] Passing parameters to Grouped Middleware(s) on Routes middleware setup #25048

Closed
wants to merge 7 commits into from

Conversation

isocroft
Copy link

@isocroft isocroft commented Aug 1, 2018

Okay, so i work for @synergixe and we had some problem reference by issue #24668 and we had to use a workaround (please see workaround code in issue referenced) which my company posted on that issue. This is a pull request that makes it easier to do the below and have all group middlewares receive the same parameters automatically.

     /* example middleware */
     namespace App\Cors;

     use \Barryvdh\Cors\HandleCors;

    class OpsCors extends HandleCors {

    }
        /* app/Http/Kernel.php */

            protected $middlewareGroups =  [
                   'security' => [
                         \App\Http\Middleware\LogStuff::class,
                         \App\Cors\OpsCors::class
                   ]
           ];
      /* routes/web.php */

     Route::middleware(['security:web'])->group(function(){
             Route::get('/model/{model_test}/', function(App\ModelTest $model_test){
                   return $model_test->getAttributes();
             });
     });

This will make the life of an average Laravel developer a lot easier going forward. Thanks

@GrahamCampbell GrahamCampbell changed the title Passing parameters to Grouped Middleware(s) on Routes middleware setup [5.6] Passing parameters to Grouped Middleware(s) on Routes middleware setup Aug 1, 2018
This pr simply updates existing code to handle cases where group middlewares should have their parameter shared amongst it individual middleware
@synergixe
Copy link

Nice work @isocroft

@GrahamCampbell
Copy link
Member

Thanks for the PR. Any chance you could add some tests so that it's clear what the code does to future readers, and to ensure it doesn't get broken in the future?

@taylorotwell
Copy link
Member

No plans to add this right now. Code is kinda gnarly.

@isocroft
Copy link
Author

isocroft commented Aug 2, 2018

@GrahamCampbell I'll be sure to add tests to the PR for proper scrutiny ASAP. Perhaps, I and @taylorotwell could work on this later on with a less challenging context to it. Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants