[5.0][Feature][Proposal] Operations to become classes instead of Traits

[tabacitu]

Operation = some action the user can perform on the Model, that could include an interface (so route, controller method, etc) but also some operation on the entry (so something to call inside the CrudPanel object);

Example: Create operation. Update operation. Delete operation. Reorder operation.

One operation can consist of:

• one/more routes;
• those routes could go to one/multiple EntityCrudController methods (called actions);
• methods and properties on the CrudPanel object;
• form validation files, or other parameters;

What I propose is that we isolate each operation into its own class, so that the definition in EntityCrudController::setup() would change to something like this:

class ProductCrudController extends CrudController
{
    public function setup()
    {
        /*
        |--------------------------------------------------------------------------
        | BASIC CRUD INFORMATION
        |--------------------------------------------------------------------------
        */
        $this->crud->setModel('App\Models\Test');
        $this->crud->setRoute(config('backpack.base.route_prefix') . '/test');
        $this->crud->setEntityNameStrings('test', 'tests');

        /*
        |--------------------------------------------------------------------------
        | BASIC CRUD INFORMATION
        |--------------------------------------------------------------------------
        */
        $createFields = [
            [ // Text
                'name' => 'title',
                'label' => "Title",
                'type' => 'text',
            ],
            [   // Textarea
                'name' => 'description',
                'label' => 'Description',
                'type' => 'textarea'
            ],
            [   // Address
                'name' => 'address',
                'label' => 'Address',
                'type' => 'address',
                // optional
                'store_as_json' => true
            ],
        ];

        $updateFields = $createFields;
        $updateFields[] = [
            [   // Textarea
                'name' => 'observations',
                'label' => 'Observations',
                'type' => 'textarea'
            ],
        ];

        $this->crud->enable(new CreateOperation($createFields));
        $this->crud->enable(new UpdateOperation($updateFields));
        $this->crud->enable(new DeleteOperation);
        $this->crud->enable(new PreviewOperation);
        $this->crud->enable(new NestOperation('title', 0));
    }
}

This enable() would:

• attach more methods to the current controller (specified inside the operation object);
• attach more methods/properties to the CrudPanel object (specified inside the operation object);

I think this could:

• make it easier to create reusable operations (create a class that extends Operation and enable it in the controller);
• make it clearer which fields are used where;
• make it easier to define stuff that's mandatory for an operation (because it's an object, configuration is passed as parameters and IDE autocomplete will help);
• make it more obvious which operations are enabled for each EntityCrudController;

[tabacitu]

We could further improve this by moving the configuration layer into its own object. So:

class ProductCrudController extends CrudController
{
    public function setup()
    {
        $this->crud = new App\Library\Backpack\CRUD\Setup\ProductCrudPanel($request);
    }
}

class ProductCrudSetup extends CrudPanel {

    public function __construct($request)
    {
        /*
        |--------------------------------------------------------------------------
        | BASIC CRUD INFORMATION
        |--------------------------------------------------------------------------
        */
        $this->setModel('App\Models\Test');
        $this->setRoute(config('backpack.base.route_prefix') . '/test');
        $this->setEntityNameStrings('test', 'tests');

        /*
        |--------------------------------------------------------------------------
        | BASIC CRUD INFORMATION
        |--------------------------------------------------------------------------
        */
        $createFields = [
            [ // Text
                'name' => 'title',
                'label' => "Title",
                'type' => 'text',
            ],
            [   // Textarea
                'name' => 'description',
                'label' => 'Description',
                'type' => 'textarea'
            ],
            [   // Address
                'name' => 'address',
                'label' => 'Address',
                'type' => 'address',
                // optional
                'store_as_json' => true
            ],
        ];

        $updateFields = $createFields;
        $updateFields[] = [
            [   // Textarea
                'name' => 'observations',
                'label' => 'Observations',
                'type' => 'textarea'
            ],
        ];

        $this->enable(new CreateOperation($createFields));
        $this->enable(new UpdateOperation($updateFields));
        $this->enable(new DeleteOperation);
        $this->enable(new PreviewOperation);
        $this->enable(new NestOperation('title', 0));
    }
}

In addition to the benefits above, this would:

• make the controller slim, instead of fat;
• solve our layering problem (right now we use EntityCrudController mostly to edit the properties of a secondary object, CrudPanel);
• allow us to pass that configuration object to other classes; so we could then create a Matrix field type, because we can pass the ProductCrudSetup object and the Matrix field type will know which fields/columns to show, what validation you need on each action, etc;

Any downsides to moving the configuration to its own object??

[tabacitu]

This is no longer needed thanks to #1937

Yes, Operations are still traits. But these traits now include the routes too. So everything you want is in one file:

• the operations (controller methods)
• the routes
• the views that are used (inside the controller methods)

You can easily enable/disable/reuse operations just by using the trait or not.

Still think the configuration file might be a good idea. But let's talk about that in its own thread, here - #1600

[tabacitu]

Eeeeh... might not have been a bad idea after all. This would make it even cleaner to create new operations. And it would allow us to enforce operations look and behave a certain way. It could also clean up the CrudPanel object.

If each Operation is a class (not an array), it would make it easier to further drill down, and add settings for a particular Action of an Operation (ex, Create operation has create() and store() - you might want to add a setting for one of those, but not both).

In this case, maybe it'd make more sense to be something like:

class CrudPanel {
    public $operations = [
        Operation, // create
        Operation, // update
        Operation, // list
    ];
}

Where:

class CreateOperation extends Operation {
    use HasFields, HasSaveAction, HasSettings;

    public $actions = ['create', 'store'];

    protected function routes($name, $controller, $options) {
        Route::get($name.'/create', [
            'as' => 'crud.'.$name.'.create',
            'uses' => $controller.'@create',
        ]);

        Route::put($name.'/create', [
            'as' => 'crud.'.$name.'.store',
            'uses' => $controller.'@store',
        ]);
    }
    public function create() {}
    public function store() {}
}

class Operation implements OperationTemplate {
     use HasSettings;
}

trait HasSettings {
    public $settings = array();

    public function set($keyName, $keyValue, $optionalActionName)
    public function get($keyName, $optionalActionName);
}

interface OperationTemplate {
    public $actions = array();
    public $settings = array();
    protected function routes($name, $controller, $options);
}

Food for thought...

[tabacitu]

If we:

• setup the routes in the controller, for each operation ([4.0][Feature][Ready] Routes are defined in each Operation #1937)
• add the ability to add settings to each operation/action ([4.0][Feature][Proposal] Ability to add a setting to a particular crud operation or action #1941)
• clean up the CrudPanel object ([4.0][Refactor][To do] Clean up CrudPanel object #1942)
• reassign the CRUD alias to the CrudPanel ([4.0][Feature][Proposal] Assign the CRUD alias/facade to CrudPanel and bind it to the service container #1988)
• make Operations classes instead of traits (like this issue proposes);
• move the getters and setters to operations (ex: HasFields trait is used by both Create and Update operations, HasColumns by both Show and List);

The new syntax, in the end, could be something like:

<?php

class ProductCrudController extends CrudController
{
    use CreateOperation, UpdateOperation, DeleteOperation, ListOperation;

    public function setup()
    {
        /*
        |--------------------------------------------------------------------------
        | BASIC CRUD INFORMATION
        |--------------------------------------------------------------------------
        */
        CRUD::setModel('App\Models\Test');
        CRUD::setRoute(config('backpack.base.route_prefix') . '/test');
        CRUD::setEntityNameStrings('test', 'tests');

        /*
        |--------------------------------------------------------------------------
        | OPERATIONS
        |--------------------------------------------------------------------------
        */

        CRUD::enable(new CreateOperation);
        CRUD::enable(new UpdateOperation);
        CRUD::enable(new DeleteOperation);
        CRUD::enable(new PreviewOperation);
        CRUD::enable(new NestOperation('title', 0));

        /*
        |--------------------------------------------------------------------------
        | BASIC CRUD INFORMATION
        |--------------------------------------------------------------------------
        */
       
        // how to configure one operation
        CRUD::operation('list')->addColumns(['title', 'description']);
        CRUD::operation('create')->addFields([
            [ // Text
                'name' => 'title',
                'label' => "Title",
                'type' => 'text',
            ],
            [   // Textarea
                'name' => 'description',
                'label' => 'Description',
                'type' => 'textarea'
            ],
            [   // Address
                'name' => 'address',
                'label' => 'Address',
                'type' => 'address',
                // optional
                'store_as_json' => true
            ],
        ]);

        // how to configure multiple operations
        CRUD::operation(['create', 'update'])->addField([ // Text
            'name' => 'title',
            'label' => "Title",
            'type' => 'text',
        ]);
        CRUD::operations(['list', 'show'])->addColumn([ // Text
            'name' => 'title',
            'label' => "Title",
            'type' => 'text',
        ]);

        // how to configure ALL operations
        CRUD::allOperations()->addFields([
            [ // Text
                'name' => 'title',
                'label' => "Title",
                'type' => 'text',
            ],
            [   // Textarea
                'name' => 'description',
                'label' => 'Description',
                'type' => 'textarea'
            ],
            [   // Address
                'name' => 'address',
                'label' => 'Address',
                'type' => 'address',
                // optional
                'store_as_json' => true
            ],
        ]);
        CRUD::allOperations()->removeField('email');


        // we could also add a magic method, so that if a method is called on the 
        // CRUD object and it doesn't exist, it would be called on all operations
        // THAT WAY, we can (also) forget allOperations(), so this would work the same:
        CRUD::addField('text');
        CRUD::addColumn('name');
    }
}

Might be better than the current syntax. Might be worse. We'd obviously encourage configuring each operation individually, but there will be people configuring stuff for ALL operations.

[tabacitu]

I think from the list of to-dos above, the following are a MUST:

• setup the routes in the controller, for each operation ([4.0][Feature][Ready] Routes are defined in each Operation #1937)
• add the ability to add settings to each operation/action ([4.0][Feature][Proposal] Ability to add a setting to a particular crud operation or action #1941)
• clean up the CrudPanel object ([4.0][Refactor][To do] Clean up CrudPanel object #1942)
• reassign the CRUD alias to the CrudPanel ([4.0][Feature][Proposal] Assign the CRUD alias/facade to CrudPanel and bind it to the service container #1988)

And the following are a COULD:

• make Operations classes instead of traits (like this issue proposes);
• move the getters and setters to operations (ex: HasFields trait is used by both Create and Update operations, HasColumns by both Show and List);

Since there are separate issues for each of the MUSTs, and this issue is specifically about the COULDs, I'm going to reprioritize this as a COULD. Let's talk about it in 5.0.