Need to register Spatie's PermissionServiceProvider

[vishalb812]

Hi,

The stock spatie functionality does not work, since their service provider does not get called. Please add the below lines in bold to Backpack\PermissionManager\PermissionManagerServiceProvider

use Spatie\Permission\PermissionServiceProvider;

class PermissionManagerServiceProvider extends ServiceProvider{ public function register() { //$this->registerPermissions(); $this->app->register(PermissionServiceProvider::class); $this->setupRoutes($this->app->router); //use this if your package has a config file // config([ // 'config/laravel-permission.php', // ]); } }

[tabacitu]

@mariusconstantin2503 ?

[vishalb812]

Hi,

FYI - I had proposed the above approach when I had taken a first look at the backpack framework, and due to lack of usage documentation for permissionmanager. Subsequently I realized that instead of adding the above entry, we can also alternatively add the spatie PermissionServiceProvider package in the providers array in app.php. Generally, I would prefer if backpack does all the dependent provider loading within its own ServiceProviders, instead of asking end-users to configure all the dependencies (like spatie/permissionmanager), but either ways is ok.

// config/app.php 'providers' => [ ... Spatie\Permission\PermissionServiceProvider::class, ];

[tabacitu]

No, you're right, your solution is better. One less line in the install process is a very good thing.

For the time being, I've updated the README to include the Spatie service provider (c757b8a78200636819e7039844d7ce676e3147f7), but in a few days I'll go across all Backpack projects and implement your solution. Let's leave the issue open until then.

Cheers!

[vishalb812]

If you are going to go ahead and load the dependent service providers, could you also load please load the dependent aliases as below?

//Load Aliases

$loader = \Illuminate\Foundation\AliasLoader::getInstance();

//For base package

$loader->alias('Alert', \Prologue\Alerts\Facades\Alert::class);

//For CRUD package

$loader->alias('CRUD', \Backpack\CRUD\CrudServiceProvider::class);
$loader->alias('Form', \Collective\Html\FormFacade::class);
$loader->alias('Html', \Collective\Html\HtmlFacade::class);

And while we are at it, to further make multi-package installation easier, a suggestion is as below -

1. In backpack/base composer.json, require all backpack packages (and their dependencies by default)

example -

"backpack/permissionmanager": "^2.0"
"spatie/laravel-permission": "^1.4",
"backpack/crud": "^2.0.0",
"backpack/generators": "^1.1",
"backpack/langfilemanager": "^1.0",
"backpack/settings": "^2.0",
"caouecs/laravel-lang" : "~3.0"
...

1. Add a new config section in backpack/base/config/base.php

'enabled_packages' => [
        'crud' => true,
        'generators' => true,
        'settings' => false
    ....
    ], 

1. In the base service provider, conditionally load the dependencies for each enabled package

if(config('backpack.base.enabled_packages.crud')){
  $this->loadCrudServiceProviderAndDependencies();
}

Advantage of this approach -

1. Simpler to implement programmatically, instead of developing a whole new backpack installer
2. Easy configuration for end users and end users need not be aware of the other backpack packages at all, other than the base package.

Drawbacks of this approach -

1. Since all backpack packages and their dependencies are included in the base composer.json, the disk space for the end user's project will be more (should be at least 100 MBs for 1 project, if all packages are included)
2. Even if a user needs to work with only 1 package (e.g. CRUD), still all the backpack packages will get downloaded on a user's machine.

Would be great if there is some way to conditionally require packages in composer.json (I am not aware if such an option exists in composer)

Still, thought I'd suggest this idea, in case this approach holds merit.

[tabacitu]

OMG. This is some great work you've done here. Proposing a solution, outlining the pros and cons, you, my sir, deserve a beer and a thank you :-)

It's one very plausible way to go, the pros and cons are clearly defined, let's ask some more people.

@mariusconstantin2503 @cristiantone @Ghitu @fede91it how do you feel about pulling all packages along with base and then just switching them on/off in a config file?

[vishalb812]

Thanks, glad you liked it. :) Another, better approach is not to touch the base package at all, but to create a new package called backpack or all, and make this as the add all package, which will include all other packages like base, pagemanager etc. The other existing packages can be left untouched. This will give flexibility to users to pick and choose packages like they currently do, or to use the single add all package if they want.

On another note, in the service providers, can you please move route registrations from the register to the boot method? That will bring things more in line with laravel recommendation to "never attempt to register any event listeners, routes, or any other piece of functionality within the register method" from https://laravel.com/docs/5.2/providers

[tabacitu]

@vishalbhide my thoughts exactly. A separate package that has everything installed has all the advantages you've mentioned and none of the drawbacks. It's simple, it's efficient, that's definitely the way to go!

What should we call it?

• backpack/example
• backpack/starter
• backpack/complete
• backpack/test

None sound very good to me. Any other ideas?

[vishalb812]

Choosing a good name is more difficult than writing code :)

A few that come to mind

backpack/whole backpack/full backpack/ultimate backpack/fusion

backpack/coco (Abbreviate for COmplete COllection) backpack/whose (Abbreviate for WHole Set)

or something totally unrelated? backpack/aurora backpack/hydra

[Ghitu]

backpack/kickstarter

backpack/all-in

Andrei Iordache Creative Director & Co-Founder

m: +40 732 512 067 e: andrei@updivision.com w: www.updivision.com

25 C.A. Rosetti Street District 2, Bucharest, Romania

Pe 16 iun. 2016, la 19:31, Cristian Tabacitu notifications@github.com a scris:

@vishalbhide https://github.com/vishalbhide my thoughts exactly. A separate package that has everything installed has all the advantages you've mentioned and none of the drawbacks. It's simple, it's efficient, that's definitely the way to go!

What should we call it?

backpack/example backpack/starter backpack/complete backpack/test None sound very good to me. Any other ideas?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Laravel-Backpack/permissionmanager/issues/3#issuecomment-226539927, or mute the thread https://github.com/notifications/unsubscribe/AHKb4owmvNaZbwfsLzs8Ye_V3-ED9Zc9ks5qMXpXgaJpZM4Iy1UC.

[tabacitu]

Hmmm... So far I think I like these best:

• backpack/full
• backpack/cms - isn't this what it is, actually, if you install all packages? A CMS for basic websites? PyroCMS v3 just launched and it's pretty much this, only with questionable UI and some more features (streams, notably);

Thoughts?

[vishalb812]

I don't think backpack should be marketed as a cms just yet, as that might raise end-user expectations - there are a few missing components/widgets like multi-language model support, sample frontend templates, galleries, basic contact form etc. As that gets added, it will be closer to a standard cms.

Maybe 2 packages could be created? backpack/full with everything excluding News and Article CRUD, and backpack/cms which includes News & Article, as well as additional components that will get added in the future?

[tabacitu]

Hmm... you may be onto something with the missing functionality to call it a CMS. Also, the CMS term has gained some negative connotation lately, better to stay away from it.

Pretty sure 2 packages is overkill. 1 package with everything installed should do.

Ok then, it's now between:

• backpack/full
• backpack/backpack (similarly to how laravel has its main package laravel/laravel)

Final list. Thoughts?

[vishalb812]

Since disk space is cheap these days, I believe there will be more users downloading the full package in the future, than pick and choose package users. My vote is for backpack/backpack

[tabacitu]

I just realised that I might have gotten excited about this solution a bit early. This all-in-one package won't be used only for testing Backpack, people will use it in production, because we're lazy and we take shortcuts - let's face it. So this will result in people having installed packages that they don't actually use, which is a very bad practice.

I think the best way to go here is to not take shortcuts: 1) For production use - Backpack/base will have a command-line installation wizard, which asks which packages you want and automatically publishes views, configs, registers service providers, runs migrations, etc. I hope this package will make it faster to implement. 2) For testing Backpack - a Laravel instance with all packages installed, called backpack/demo. This way, it's easy to test, but it's obviously not for production use.

[vishalb812]

Ok. backpack/demo option is good as well.