[4.0][Proposal] Maybe move CrudPanel configuration to its own object?

[tabacitu]

Right now we're configuring the entire CrudPanel object inside the EntityCrudController::setup() method. That's basically all we're doing inside setup(), because all calls start with $this->crud.

I think this makes Backpack easy to understand. Since it happens inside the EntityCrudController, even a first-timer with medium OOP knowledge should understand what's happening.

BUT.

For big (huge) CRUDs, the setup() method becomes so bloated. Just take a look at the MonsterCrudController in the demo. It's very very rare that you'll need a CRUD this complicated, but when you do, it's a drag to organize the code inside the CrudController. There are a few options, but none of them good, in my opinion. You can further split your configuration using:

1. comments inside the setup() method (not really an organization method imho)
2. methods like addFieldsToCrud(), addFiltersToCrud() or something like addMetaFakeFields() that you could even isolate into a trait, that you can then reuse across multiple EntityCrudControllers;

Out of the above, I definitely like no 2 better, but even that one has got problems:

• I'm used to my controllers having all methods public and accessible through routes.
• When creating new methods, you really have to make sure you're not overwriting some CrudPanel or CrudTrait methods;
• The CrudPanel configuration is in the same object as its representation;

A solution to this problem (or maybe it isn't a real problem, you tell me) could be to configure the CrudPanel separately - not in the CrudPanel at all, but in a Builder. So that your MonsterCrudController could look something like:

<?php

namespace App\Http\Controllers\Admin;

use App\Http\Requests\MonsterRequest as StoreRequest;
// VALIDATION: change the requests to match your own file names if you need form validation
use App\Http\Requests\MonsterRequest as UpdateRequest;
use Backpack\CRUD\app\Http\Controllers\CrudController;
use App\CrudPanel\MonsterCrudPanelConfiguration;

class MonsterCrudController extends CrudController
{
    public function setup()
    {
         $this->crud = $this->crud->makeFrom(MonsterCrudPanelConfiguration::class);
    }

    public function store(StoreRequest $request)
    {
        $redirect_location = parent::storeCrud($request);

        return $redirect_location;
    }

    public function update(UpdateRequest $request)
    {
        $redirect_location = parent::updateCrud($request);

        return $redirect_location;
    }

    public function show($id)
    {
        $content = parent::show($id);

        // so we want to change something only for the Show operation
        // ok, sure, we can do it here
        $this->crud->addColumn([
            'name' => 'table',
            'label' => 'Table',
            'type' => 'table',
            'columns' => [
                'name'  => 'Name',
                'desc'  => 'Description',
                'price' => 'Price',
            ]
        ]);
        $this->crud->addColumn([
            'name' => 'fake_table',
            'label' => 'Fake Table',
            'type' => 'table',
            'columns' => [
                'name'  => 'Name',
                'desc'  => 'Description',
                'price' => 'Price',
            ],
        ]);
        $this->crud->addColumn([
            'name' => 'upload_multiple',
            'label' => 'Upload multiple',
            'type' => 'upload_multiple',
            // 'disk' => 'uploads',
        ]);
        $this->crud->addColumn('text');
        $this->crud->removeColumn('date');
        $this->crud->removeColumn('extras');

        return $content;
    }
}

With a separate file containing the actual configuration (pseudo-code here):

<?php

namespace App\CrudPanels;

class MonsterCrudControllerConfiguration extends CrudPanelConfiguration 
{
	public function fields() {
		return [];
	}

	public function columns()
	{
		return [];
	}

	public function filters()
	{
		return [];
	}

	public function buttons()
	{
		return [];
	}
}

What I like about this:

• would make it easier for someone to see all configuration options, since the methods are generated in the builder;
• would probably make it easier to generate configurations;
• it would separate CrudPanel configuration from the actual representation; we need to do that anyway, at some point - it's just good OOP for big objects like this one;
• it would allow us to create CRUDs inside CRUDs (aka "matrix field type", aka “add related item in a pop-up”) - we could just have a related_crud field type that points to this configuration file;
• it would make it easier for us to generate 100% complete CRUD panels - we just generate this configuration file;
• it would make it easier to share configurations between admin panels, or create multiple admin panels for the same entity (just extend the configuration file and modify a few things);

What I don't like about this solution:

1. configuration is still not split up into Operations (Create, Update, Delete, ListEntries), and I think it should be;
2. how would you go about enabling/disabling some nitch features of an operation? say disableResponsiveTable()?

Food for thought...

[tabacitu]

Following up on this. Here's how a configuration file might look like:

Solution A

This solution does NOT fix problem 1. Configuration is still not split by operation - I find that a bit ugly.

This solution fixes problem 2 above (easy to enable / disable / configure nitch features of an operation) by having a $settings property where you define all that. Not the sexiest solution, but an array consisting of operation.feature.setting keys would be pretty easy to understand and work with, imho.

Solution B

This solution fixes problem 1, it has all configurations split into operations - each operation has its own method, which returns all configurations for that operation. This also makes it easy and intuitive to re-use configurations across operations (just call that method and use the part of the array you’re interested in).

This solution also fixes problem 2. It’s easy and intuitive to enable/disable/configure nitch features for an operation, because you can return whatever you want in settings for that operation, and that operation alone.

—

You can find both proposed files in this gist, but I think it’s easier to look at the screenshots above. Both files have all configurations as the demo MonsterCrudController, which is… a monster. A CRUD will rarely get this big and ugly. But when it does, I think it should be easier to organize.

I quite like solution B.

Thoughts?

[tabacitu]

I think this PR #1937 makes it possible to pass the CrudController to a field, and it will have everything inside it:

• routes
• fields
• columns

It would also probably be cleaner to declare, inside the usual setup() method, like outlined here - #1578 (comment) . Each setting is declared PER OPERATION, instead of globally. Which, I think, becomes very relevant when you enable a lot of Operations for a Model.

[tabacitu]

Let's scrap/postpone this idea, since PR #1937 and PR #2000 provide a good, mandatory solution for splitting the CrudPanel definition:

Solution 1

    // ...
    public function setup() {
        // ...
        CRUD::operation(['create', 'update'], function() {
            CRUD::addField([
                'name' => 'name',
                'label' => 'Name',
            ]);
            CRUD::addField([
                'name' => 'slug',
                'label' => 'Slug (URL)',
                'type' => 'text',
                'hint' => 'Will be automatically generated from your name, if left empty.',
                // 'disabled' => 'disabled'
            ]);
            CRUD::addField([
                'label' => 'Parent',
                'type' => 'select',
                'name' => 'parent_id',
                'entity' => 'parent',
                'attribute' => 'name',
                'model' => "Backpack\NewsCRUD\app\Models\Category",
            ]);
        });
        // ...
    }
    // ...

Solution 2

    // ...
    public function setup() {
        // ...
    }

    public function setupCreateConfiguration() {
            CRUD::addField([
                'name' => 'name',
                'label' => 'Name',
            ]);
            CRUD::addField([
                'name' => 'slug',
                'label' => 'Slug (URL)',
                'type' => 'text',
                'hint' => 'Will be automatically generated from your name, if left empty.',
                // 'disabled' => 'disabled'
            ]);
            CRUD::addField([
                'label' => 'Parent',
                'type' => 'select',
                'name' => 'parent_id',
                'entity' => 'parent',
                'attribute' => 'name',
                'model' => "Backpack\NewsCRUD\app\Models\Category",
            ]);
    }

    public function setupUpdateConfiguration() {
        $this->setupCreateConfiguration();
        // anything you'd like different from the Create configuration can be set up here
    }
    // ...

Two solutions already - I don't think we need yet another solution to the same problem. I might be wrong though, if anybody stumbles upon this thread and feels differently - let me know and we could re-open the issue.

Cheers!