PermissionManager icon indicating copy to clipboard operation
PermissionManager copied to clipboard

php artisan backpack:extend-permissionmanager

Open tabacitu opened this issue 8 years ago • 9 comments

A command that makes overwriting default functionality easier.

An artisan command that creates blank controllers, requests and models. Those classes:

  • would be generated inside the respective project folders;
  • would extend the classes in Backpack;
  • would have the proper namespace;

So that the developer would only need to write the logic for the method they want to overwrite.

tabacitu avatar Sep 29 '16 06:09 tabacitu

Hey @tabacitu , when I forked and did this for my project I just add this to PermissionManagerServiceProvider: $this->publishes([__DIR__.'/app/Http/Controllers' => app_path('Http/Controllers/Permissions')], 'permission-controllers'); $this->publishes([__DIR__.'/app/Http/Requests' => app_path('Http/Requests/Permissions')], 'permission-requests'); $this->publishes([__DIR__.'/app/Models' => app_path('Models/Permissions')], 'permission-models');

I altered the default controllers and requests to have the right namespaces though and also altered the route group namespace, how would you setup this to cater for both cases? If its simple im happy to alter and tidy up what ive done and submit a PR.

b8ne avatar Oct 02 '16 04:10 b8ne

I guess the proper procedure would be very similar, but automated. So the commands wouldn't be defined as "publish" commands, as they would need to do some extra stuff too. I think the best place to put a command like this would be inside src\Console\Commands\ExtendPermissionManagerCommand.php, like we do in Backpack\Generators, to keep the Laravel convention. That command can then be easily registered inside PermissionManagerServiceProvider.

Then the ExtendPermissionManagerCommand would do things very similar to how Backpack\Generator does with the Controller/Model/Request stubs:

  • publish some blank controllers, requests, models, that extend the ones in Backpack; I personally would have all their methods there, calling their parents, so it would encourage people do things before or after the parent call, if possible, not overwrite the entire functionality; this way they'd also benefit from updates;
  • replace Backpack\ with AppNamespace\ everywhere inside the copied stubs, where AppNamespace can be determined this way (we can replace it the same way generators do with table name);
  • give success/error messages to the developer in the command line (x published, y published, z published);
  • remind the developer in the command line that he now needs to setup the routes that point to these new controllers and give him the code so he can copy-paste:
CRUD::resource('permission', 'PermissionCrudController');
CRUD::resource('role', 'RoleCrudController');
CRUD::resource('user', 'UserCrudController');

To sum up, honestly, I think the code is pretty different. It's more work to automate it, so I don't think merging with your fork makes sense right now. I would be very very happy to merge, however, if you did it this automated way :-) It would be a big help for me personally and I think, for hundreds/thousands others that use the package.

Cheers!

tabacitu avatar Oct 02 '16 09:10 tabacitu

~~Since I need this exact functionality I will look into this tomorrow, fork it and create a PR once it works. So basically, feel free to assign it to me ;-)~~

Simply extending the BP Role and Permission Class within my own and using the config file to adjust the table names has already gotten me pretty far. Going to need a bit more time to look into this properly.

datune avatar Dec 25 '16 22:12 datune

I wanted to extend the Controller, Request & Model.

However, The CRUD Controller throw me the exception for the extended class from original PermissionCrudRequest

Declaration of App\Http\Controllers\Admin\PermissionCrudController::store(App\Http\Requests\PermissionRequest $request) should be compatible with Backpack\PermissionManager\app\Http\Controllers\PermissionCrudController::store(Backpack\PermissionManager\app\Http\Requests\PermissionCrudRequest $request)

kiddtang avatar Oct 22 '17 09:10 kiddtang

@kiddtang your PermissionRequest should inherit from the Backpack PermissionCrudRequest to fix this.

OliverZiegler avatar Nov 18 '17 13:11 OliverZiegler

@OliverZiegler I did that but the Warning still there...

I am overriding authorize() method for my customized guard

namespace App\Http\Requests;

use Backpack\PermissionManager\app\Http\Requests\PermissionCrudRequest as BackpackPermissionCrudRequest;
use Illuminate\Foundation\Http\FormRequest;

class PermissionRequest extends BackpackPermissionCrudRequest
{
    /**
     * Determine if the user is authorized to make this request.
     *
     * @return bool
     */
    public function authorize()
    {
        return \Auth::guard('admin')->check();
    }

}

My Controller then use the PermissionRequest

namespace App\Http\Controllers\Admin;

use Backpack\PermissionManager\app\Http\Controllers\PermissionCrudController as BackpackPermissionCrudController;

use App\Http\Requests\PermissionRequest as StoreRequest;
use App\Http\Requests\PermissionRequest as UpdateRequest;

class PermissionCrudController extends BackpackPermissionCrudController
{
    public function __construct()
    {
        parent::__construct();
    }

    public function store(StoreRequest $request)
    {
        return parent::storeCrud();
    }

    public function update(UpdateRequest $request)
    {
        return parent::updateCrud();
    }
}

kiddtang avatar Dec 03 '17 06:12 kiddtang

@kiddtang Have you solved your issue , because i am facing the exact same error in my project..

abdelrahmanahmed avatar Jan 14 '18 11:01 abdelrahmanahmed

@abdelrahmanahmed I modify the package source file PermissionCrudController.php

    public function __store(StoreRequest $request)
    {
        return parent::storeCrud();
    }

    public function __update(UpdateRequest $request)
    {
        return parent::updateCrud();
    }

So my function could override the default behavior. Hope it could help you. I am not sure it is the right way or not, but at least there's no issue on my project so far.

kiddtang avatar Jan 24 '18 09:01 kiddtang

I'm also facing this issue. But I don't like the idea to manually overwrite the vendor files. What I did was to write the validations inline, inside the extended controller.

You can accomplish that by just writing inside the store and update methods:

$request->validate([
    'name' => [
        'required',
        'min:10',
    ],
]);

Of course, you can extract your validation to another method and call it instead of writing this twice.

claudsonm avatar Jul 17 '18 22:07 claudsonm

Hey everyone! I no longer think this is worth doing. I think it's one of those cases where clear documentation eliminates the need for a feature. And I think this section provides clear instructions on how to customize any class this package provides. Plus, we shouldn't make it too easy for people to extend this and customize... if they do that without knowing what they're doing... that's a recipe for disaster 😔

So I'm going to close the issue. This wasn't a priority anyway, so it's unlikely we would get to it this year either. Better closed than "never done".

Let me know if you disagree. I'm wrong at least once a day, like everybody else 😀

Cheers!

tabacitu avatar Oct 11 '23 12:10 tabacitu