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

[Feature proposal] Fluent syntax for CRUD #394

Closed
tabacitu opened this issue Jan 27, 2017 · 24 comments
Closed

[Feature proposal] Fluent syntax for CRUD #394

tabacitu opened this issue Jan 27, 2017 · 24 comments
Assignees

Comments

@tabacitu
Copy link
Member

tabacitu commented Jan 27, 2017

I think I just had an epiphany, because all these changes seem excellent to me AND I think they're backwards-compatible:

(1) What if we use a Facade for the CRUD, and use CRUD::addField() instead of $this->crud->addField()? Shorter is better, right?

(2) What if we also create a fluent syntax for defining fields, columns, etc, like Laravel 5.4 does with its routes (Route::name('users.index')->middleware('auth');)?

We could then use both this:

<?php namespace App\Http\Controllers\Admin;

use Backpack\CRUD\app\Http\Controllers\CrudController;

// VALIDATION: change the requests to match your own file names if you need form validation
use App\Http\Requests\ArticleRequest as StoreRequest;
use App\Http\Requests\ArticleRequest as UpdateRequest;

class ArticleCrudController extends CrudController {

	public function setup() {

        /*
		|--------------------------------------------------------------------------
		| BASIC CRUD INFORMATION
		|--------------------------------------------------------------------------
		*/
        $this->crud->setModel("App\Models\Article");
        $this->crud->setRoute("admin/article");
        $this->crud->setEntityNameStrings('article', 'articles');

        /*
		|--------------------------------------------------------------------------
		| COLUMNS AND FIELDS
		|--------------------------------------------------------------------------
		*/

        // ------ CRUD COLUMNS
        $this->crud->addColumn([
                                'name' => 'date',
                                'label' => 'Date',
                                'type' => 'date'
                            ]);
        $this->crud->addColumn([
                                'name' => 'status',
                                'label' => "Status"
                            ]);
        $this->crud->addColumn([
                                'name' => 'title',
                                'label' => "Title"
                            ]);
        $this->crud->addColumn([
                                'name' => 'featured',
                                'label' => "Featured",
                                'type' => "model_function",
                                'function_name' => 'getFeaturedColumn'
                            ]);
        $this->crud->addColumn([
                                'label' => "Category",
                                'type' => 'select',
                                'name' => 'category_id',
                                'entity' => 'category',
                                'attribute' => 'name',
                                'model' => "App\Models\Category"
                            ]);

        // ------ CRUD FIELDS
        $this->crud->addField([    // TEXT
                                'name' => 'title',
                                'label' => 'Title',
                                'type' => 'text',
                                'placeholder' => 'Your title here'
                            ]);
        $this->crud->addField([
                                'name' => 'slug',
                                'label' => "Slug (URL)",
                                'type' => 'text',
                                'hint' => 'Will be automatically generated from your title, if left empty.'
                                // 'disabled' => 'disabled'
                            ]);

        $this->crud->addField([    // TEXT
                                'name' => 'date',
                                'label' => 'Date',
                                'type' => 'date',
                                'value' => date('Y-m-d')
                            ], 'create');
        $this->crud->addField([    // TEXT
                                'name' => 'date',
                                'label' => 'Date',
                                'type' => 'date'
                            ], 'update');

        $this->crud->addField([    // WYSIWYG
                                'name' => 'content',
                                'label' => 'Content',
                                'type' => 'ckeditor',
                                'placeholder' => 'Your textarea text here'
                            ]);
        $this->crud->addField([    // Image
                                'name' => 'image',
                                'label' => 'Image',
                                'type' => 'browse'
                            ]);
        $this->crud->addField([    // SELECT
                                'label' => "Category",
                                'type' => 'select2',
                                'name' => 'category_id',
                                'entity' => 'category',
                                'attribute' => 'name',
                                'model' => "App\Models\Category"
                            ]);
        $this->crud->addField([    // Select2Multiple = n-n
                                'label' => 'Tags',
                                'type' => 'select2_multiple',
                                'name' => 'tags',
                                'entity' => 'tags',
                                'attribute' => 'name',
                                'model' => "App\Models\Tag",
                                'pivot' => true,
                            ]);
        $this->crud->addField([    // ENUM
                                'name' => 'status',
                                'label' => "Status",
                                'type' => 'enum'
                            ]);
        $this->crud->addField([    // CHECKBOX
                                'name' => 'featured',
                                'label' => "Featured item",
                                'type' => 'checkbox'
                            ]);
    }

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

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

AND this:

<?php namespace App\Http\Controllers\Admin;

use Backpack\CRUD\app\Http\Controllers\CrudController;

// VALIDATION: change the requests to match your own file names if you need form validation
use App\Http\Requests\ArticleRequest as StoreRequest;
use App\Http\Requests\ArticleRequest as UpdateRequest;

class ArticleCrudController extends CrudController {

    public function setup() {
        /*
        |--------------------------------------------------------------------------
        | BASIC CRUD INFORMATION
        |--------------------------------------------------------------------------
        */
        CRUD::setModel("App\Models\Article");
        CRUD::setRoute("admin/article");
        CRUD::setEntityNameStrings('article', 'articles');

        /*
        |--------------------------------------------------------------------------
        | COLUMNS AND FIELDS
        |--------------------------------------------------------------------------
        */

        // ------ CRUD COLUMNS
        CRUD::addColumn('date')->label('Date')->type('date');
        CRUD::addColumn('status')->label('Status');
        CRUD::addColumn('name')->label('Title');
        CRUD::addColumn('featured')->label('Featured')->type('model_function')->function_name('getFeaturedColumn');
        CRUD::addColumn('category_id')->type('select')->label('Category')->entity('category')->attribute('name')->model('App\Models\Category');

        // ------ CRUD FIELDS
        CRUD::addField('title')->label('Title')->type('text')->placeholder('Your title here');
        CRUD::addField('slug')->label('Slug (URL)')->type('text')->hint('Will be automatically generated from your title, if left empty');
        CRUD::addField('date')->label('Date')->type('date')->value(date('Y-m-d'))->showOnCreate(); // alternatives: showOnCreate(); showOnUpdate(); showOnBoth(); hideOnCreate(); hideOnUpdate();
        CRUD::addField('date')->label('Date')->type('date')->showOnUpdate();
        CRUD::addField('content')->label('Content')->type('ckeditor')->placeholder('Your textarea text here');
        CRUD::addField('image')->label('Image')->type('browse');
        CRUD::addField('category_id')->type('select2')->label('Category')->entity('category')->attribute('name')->model('App\Models\Category');
        CRUD::addField('tags')->type('select2_multiple')->label('Tags')->entity('tags')->attribute('name')->model('App\Models\Tag')->pivot(true);
        CRUD::addField('name')->label('Status')->type('enum');
        CRUD::addField('featured')->label('Featured item')->type('checkbox');
    }

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

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

I'm super-excited about this, because

  • it looks and works more Laravely
  • it would help us use the IDE's autocomplete more
  • I personally like it better, because it makes for shorter controllers

but I don't exactly have strong technical reasons... Thoughts?

@tabacitu tabacitu added this to the 3.3 - extras milestone Jan 27, 2017
@tabacitu tabacitu self-assigned this Jan 27, 2017
@OwenMelbz
Copy link
Contributor

OwenMelbz commented Jan 27, 2017

I'll keep this one short and sweet :D

  1. I like it.
  2. It seems much longer to write.
  3. Cant use the mass assignment styles ones like addFields?
  4. As long as we obviously keep full backwards compat
  5. How easy will it be to add your own field types and still read the attributes? or will it literally copy the called method as an attribute?
  6. Could work with the withFilters->withCake->withSearch etc on routes
  7. Definitely late 3.2 or 3.3 as think theres more key features to get in first :D
  8. How would it work if people are using extended/customised Cruds (e.g this->crud - is not the vendor version)

@tabacitu
Copy link
Member Author

  1. Great!
  2. Might be. But you could skip a bunch of stuff that's auto-generated (label, type, entity, etc).
  3. Hmm... don't know... Can't see a way, honestly.
  4. Of course. This will just create the array for you. It would be an alternative syntax.
  5. Same process. It would literally copy the method as an attribute.
  6. Agreed.
  7. Definitely not 3.2. Hopefully 3.3 or even 4.0. Not a priority, agreed.
  8. We could let them specify another class the Facade wraps around. Either in a config file or just have the Facade inside the app folder directly.

@OliverZiegler
Copy link
Contributor

I had a similar idea wrapping all field and column types in classes to even provide exact code completion for supported attributes depending on field/column type. This would be very helpful when starting to use backpack. And syntax would also be more Laravely.

$date = DateCRUDField::name('date')->label('date')->type('date')->value(date('Y-m-d'))->showOnCreate();
$select2 = new Select2CRUDField('category_id')->type('select2')->label('Category')->entity('category')->attribute('name')->model('App\Models\Category');
        
$this->crud->addField($date);
$this->crud->addField($select2);

or even

$this->crud->addFields([$date, $select2]);

When we want to stick to an Laravel Route equivalent we could also do something like

$date = CRUDField::date('date')->label('date')->value(date('Y-m-d'));
$select2 = CRUDField::select2('category_id')->label('Category')->entity('category')->attribute('name')->model('App\Models\Category');

And instead of real classes just return some contract to get intelligent code completion depending on field/column type.

What do you think?

@tabacitu
Copy link
Member Author

tabacitu commented May 8, 2017

Hi @OliverZiegler ,

I think CRUDField::date('date') is definitely more OOP than CRUD::addField('date')->type('date'), yes. And would definitely help with the API (suggestions on the CRUDField class would be short, while suggestions on the CRUD class will be the full API).

One problem I see, though, is the name convention. I really don't like prefixing or suffixing all classes with CRUD, looks bad and it is confusing (in case we split it up, some methods will not be for all actions - CREATE, READ, UPDATE, DELETE, but just for a few of them. Yet you type CRUD for each of them). Maybe we find a solution together. Say:

// general
Panel::setModel("App\Models\Example");
Panel::setRoute("admin/example");
Panel::setRouteName("admin.example");
Panel::setEntityNameStrings("example", "examples");

Fields::setFromDb();  // automatically set fields and columns from your database columns
Columns::setFromDb();

// ------ FIELDS
Fields::add($field_definition_array)->forAction('update/create/both');
Fields::add($array_of_fields_definition_arrays)->forAction('update/create/both');
Fields::remove('name')->forAction('update/create/both');
Fields::remove($array_of_names->forAction('update/create/both');

// ------ COLUMNS
Columns::add(); // add a single column, at the end of the stack
Columns::add([]); // add multiple columns, at the end of the stack
Columns::remove('column_name'); // remove a column from the stack
Columns::remove(['column_name_1', 'column_name_2']); // remove an array of columns from the stack
Columns::setDetails('column_name', ['attribute' => 'value']);
Columns::setDetails(['column_1', 'column_2'], ['attribute' => 'value']);

Columns::set(); // set the columns you want in the table view, either as array of column names, or multidimensional array with all columns detailed with their types

// ------ REORDER FIELDS
Fields::add()->before('name'); // will show this before the given field
Fields::add()->after('name'); // will show this after the given field

// ------ REORDER COLUMNS
Columns::add()->before('name'); // will show this before the given column
Columns::add()->after('name'); // will show this after the given column


// ------ CRUD BUTTONS
// possible positions: 'beginning' and 'end'; defaults to 'beginning' for the 'line' stack, 'end' for the others;
Buttons::add($stack, $name, $type, $content, $position); // add a button; possible types are: view, model_function
Buttons::addFromModelFunction($stack, $name, $model_function_name, $position); // add a button whose HTML is returned by a method in the CRUD model
Buttons::addFromView($stack, $name, $view, $position); // add a button whose HTML is in a view placed at resources\views\vendor\backpack\crud\buttons
Buttons::remove($name);
Buttons::removeFromStack($name, $stack);


// ------ ACCESS
Panel::allowAccess('list');
Panel::allowAccess(['list', 'create', 'delete']);
Panel::denyAccess('list');
Panel::denyAccess(['list', 'create', 'delete']);

Panel::hasAccess('add'); // returns true/false
Panel::hasAccessOrFail('add'); // throws 403 error
Panel::hasAccessToAll(['create', 'update']); // returns true/false
Panel::hasAccessToAny(['create', 'update']); // returns true/false


// ------ REORDER
Reorder::enable('label_name', MAX_TREE_LEVEL);
// NOTE: you also need to do allow access to the right users: $this->crud->allowAccess('reorder');

Reorder::disable();
Reorder::isEnabled(); // return true/false


// ------ DETAILS ROW
Panel::enableDetailsRow();
// NOTE: you also need to do allow access to the right users: $this->crud->allowAccess('details_row');
// NOTE: you also need to do overwrite the showDetailsRow($id) method in your EntityCrudController to show whatever you'd like in the details row OR overwrite the views/backpack/crud/details_row.blade.php

Panel::disableDetailsRow();

// ------ REVISIONS
// You also need to use \Venturecraft\Revisionable\RevisionableTrait;
// Please check out: https://laravel-backpack.readme.io/docs/crud#revisions
Panel::allowAccess('revisions');

// ------ AJAX TABLE VIEW
// Please note the drawbacks of this though: 
// - 1-n and n-n relationship columns won't be searched anymore;
// - date and datetime columns won't be sortable anymore;
Panel::enableAjaxTable(); 

// ------ DATATABLE EXPORT BUTTONS
// Show export to PDF, CSV, XLS and Print buttons on the table view.
// Does not work well with AJAX datatables.
Panel::enableExportButtons();

// ------ ADVANCED QUERIES
Panel::addClause('active');
Panel::addClause('type', 'car');
Panel::addClause('where', 'name', '=', 'car');
Panel::addClause('whereName', 'car');
Panel::addClause('whereHas', 'posts', function($query) {
     $query->activePosts();
 });
Panel::orderBy();
Panel::groupBy();
Panel::limit();

Etc. Do you see any problems to this? Think it would conflict with other class names?

@OwenMelbz
Copy link
Contributor

If you were worried about naming conflicts, you could keep the classes all CRUDPanel:setModel

But when you import the class alias it?

use Backpack\CrudPanel as Panel;

@tabacitu
Copy link
Member Author

@OwenMelbz so true. That's a simple solution :-)

@OliverZiegler
Copy link
Contributor

Is there any progress on this?

I'm currently wondering how to achieve this as a standalone package as I can't remember all the fields and columns and hate always looking them up in the docs.

How could this Columns::remove('column_name'); work as we have no reference to the current CRUDPanel?

My suggestion (for a first standalone package) would be:

  • offering all the Columns and Fields in separate classes that can be imported
    for example: CRUDField
$date = CRUDField::date('date')->label('date')->value(date('Y-m-d'));
$select2 = CRUDField::select2('category_id')->label('Category')->entity('category')->attribute('name')->model('App\Models\Category');
  • calling some function like generate would return the array like we use it currently
  • now we can use the old $this->crud->addField function

For Backpack updates we could just use the addField function to accept a CRUDField and if so we call generate inside.

What do you think?

@tabacitu
Copy link
Member Author

tabacitu commented Aug 4, 2017

@OliverZiegler I was thinking we could use Facades for that. Similar to how you can do App::smth or $this->app->smth.

@OliverZiegler
Copy link
Contributor

@tabacitu Sounds good.

Will have a closer look on how facades work and start working on it in a fork.
(Fields and Columns first I suggest)

Any more input on this I should know?

@tabacitu
Copy link
Member Author

tabacitu commented Aug 4, 2017

Not really, no. Ideally all these Facades should point to the same object - $crudPanel - so that we can make this backwards-compatible.

@OliverZiegler
Copy link
Contributor

Ok. Will tell you when there is code to review

@lloy0076
Copy link
Contributor

lloy0076 commented Aug 4, 2017

👿 Devil's Advocate WARNING

What would this improve over other PRs (such as merging in all the fixes to data tables etc)?

@OliverZiegler
Copy link
Contributor

OliverZiegler commented Aug 4, 2017

@lloy0076 not sure what you mean.

I think this would help new users get started easier and speed up your own process as you don't need to remember everything or look it up in the docs 😉

@tabacitu
Copy link
Member Author

tabacitu commented Aug 4, 2017

@lloy0076 - apples and oranges. I expect ALL existing PRs to be merged or closed in the next 1-2 weeks, as we prepare for the next breaking change. Bold plan, I know :-)

@OliverZiegler
Copy link
Contributor

Started implementing this here.

Currently available Facades:

  • CRUDPanel to replace $this->crud calls
  • CRUDColumns to create and manage columns in a more laravely way

What do you think? Should I proceed like this for the fields?

@tabacitu
Copy link
Member Author

tabacitu commented Aug 9, 2017

Hi @OliverZiegler - I've bundled it into a PR so it's easier to see the difference - https://github.com/Laravel-Backpack/CRUD/pull/826/files - but won't merge it until it's ready (of course).

Thanks for the short feedback loop. I'll take a look at it ASAP and come back to you - lots to think about and ponder about this...

@eduardoarandah
Copy link
Contributor

My two cents here:

For every new concept introduced, there's a cost in learning.
Human learning is expensive, fingers are slow.

My opinion is: keep it simple, and then, make it simpler.

This would be good:


$this->crud->columns([
    [
        'name'  => 'date',
        'label' => 'Date',
        'type'  => 'date',
    ],
    [
        'name'  => 'status',
        'label' => "Status",
    ],
    [
        'name'  => 'title',
        'label' => "Title",
    ],
    [
        'name'  => 'birth_date',
        'label' => "Birth date",
    ],
]);

How to make it simpler?
If label is not defined, uppercase first letter and replace _ for spaces
birth_date becomes Birth date

$this->crud->columns([
    [
        'name'  => 'date',
        'type'  => 'date',
    ],
    [
        'name'  => 'status',
    ],
    [
        'name'  => 'title',
    ],
    [
        'name'  => 'birth_date',
    ],
]);

How to make it simpler?
check if it's an array, if not, assume it's the field name

look at this beauty:

$this->crud->columns([
    'date',
    'status',
    'title',
    'birth_date'
]);

@OwenMelbz
Copy link
Contributor

@eduardoarandah thats basically what it does now xD

$this->crud->addColumns([
    ['name' => 'name'],
    ['name' => 'email'],
    ['name' => 'phone'],
]);

@eduardoarandah
Copy link
Contributor

really? then it's perfect

does it accept a single string like this? haven't checked

$this->crud->columns([
    'name',
    'email',
    'phone'
]);

@OliverZiegler
Copy link
Contributor

I think this currently won't work.
There is some case handling where if name is missing the columns will get an anonymous name but then it will miss the label...

The intention behind this feature was, that your IDE can help you remember different column/field types by providing the options via code completion...
In my case I always have to log up the more special stuff in the docs... would be so much faster if my IDE would just show me alle the possibilities ;)

@lloy0076
Copy link
Contributor

Yes, but I don't use an IDE and there are at 80+ open issues and the AJAX functionality is still, at best, wobbly...

@lloy0076
Copy link
Contributor

@tabacitu @OliverZiegler @eduardoarandah - this is a good idea but until "someone" has the time to implement it, it's cluttering up the issue boards.

@OliverZiegler
Copy link
Contributor

Started implementing this in #826 but waiting for some feedback for a while now.

@lloy0076
Copy link
Contributor

lloy0076 commented Feb 9, 2018

I'm going to close this in favour of discussion in #826.

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

No branches or pull requests

5 participants