Crud: [Feature request] Support callbacks to default fieldtypes

[thplat]

After researching the issue yesterday I found out that Backpack Crud currently does not support filtering results / customizing queries for the default field types.

I would have expected that a callback can be assigned to the field whose result is then used in the view.

Currently it seems that there are not much options to do that. One option is to extend the Model in question and add global state to it, and the other one is using custom field types.

Both do not seem to be very clean to me, and I would suggest having callbacks added to the fields which are used if they exist.

To clarify my issue let's assume we want to use a select fieldtype to choose from Roles and for simplicity we want only the Superadmin role returned. So our field would be added like this:

$this->crud->addFields([  // Select
    'label' => "Role",
    'type' => 'select_callback', // custom fieldtype for demonstration purposes
    'name' => 'id', // the db column for the foreign key
    'entity' => 'roles', // the method that defines the relationship in your Model
    'attribute' => 'name', // foreign key attribute that is shown to user
    'model' => config('laravel-permission.models.role'), // foreign key model,
    'callback' => function() {
        return Role::where('name', 'Superadmin')->get();
    }
]);

For the select_callback view I took the select view and changed only one line there.
We go from: @foreach ($field['model']::all() as $connected_entity_entry) to @foreach ((isset($field['callback']) ? $field['callback']() : $field['model']::all()) as $connected_entity_entry).

The resulting select field will now only have the Superadmin item in it. A similar logic could be attached by default to all fieldtypes where it makes sense.

If you consider this as a somewhat pretty solution I'd be happy to submit a PR for that.

[tabacitu]

Hi @thplat ,

I love it. Let's schedule this as one of the next features. In my eyes, it only makes sense for fields that use relationships (select, select2, select2_multiple, etc). [Q1] Would you agree? Do you see any other use case I missed?

Also, I tried to find other possible use cases than basically filtering or modifying a set of data. And I found none. So I think a more Laravely way to frame this is a "scope". Just like models have scopes, your fields can have scopes. Otherwise the definition of the entity would happen in two places:

• when you define the entity, attribute, model
• inside the callback

and it could be confusing - you wouldn't know which one of them is actually working. [Q2] I think framing this feature as "scopes" solves that, as you define the entity "the old way", then just reference that one inside the anonymous method. Would you agree?

So I'm leaning toward something more like this:

$this->crud->addFields([  // Select
    'label' => "Role",
    'type' => 'select_callback', // custom fieldtype for demonstration purposes
    'name' => 'id', // the db column for the foreign key
    'entity' => 'roles', // the method that defines the relationship in your Model
    'attribute' => 'name', // foreign key attribute that is shown to user
    'model' => config('laravel-permission.models.role'), // foreign key model,
-    'callback' => function() {
-        return Role::where('name', 'Superadmin')->get();
-    }
+    'scope' => function($model) {
+       return $model->where('name', 'Superadmin');
+    }
]);

And then:

- @foreach ((isset($field['callback']) ? $field['callback']() : $field['model']::all()) as $connected_entity_entry)
+ @foreach ((isset($field['callback']) ? $field['callback']()->get() : $field['model']::all()) as $connected_entity_entry)

What do you think about my two questions above? Good improvement on an already good solution, or off-based?

Thanks, cheers!

[tabacitu]

Hmm... that being said, we could also support existing scopes. So if you define the scope as an anonymous method, we would run that. Otherwise, if it's text, we could use the scope with that name.

So for:

class Role extends Model
{
    public function scopeSuperadmin($query)
    {
        return $query->where('name', 'Superadmin');
    }
...

we could do:

$this->crud->addFields([  // Select
    'label' => "Role",
    'type' => 'select_callback', // custom fieldtype for demonstration purposes
    'name' => 'id', // the db column for the foreign key
    'entity' => 'roles', // the method that defines the relationship in your Model
    'attribute' => 'name', // foreign key attribute that is shown to user
    'model' => config('laravel-permission.models.role'), // foreign key model,
-    'scope' => function($model) {
-       return $model->where('name', 'Superadmin');
-    }
+    'scope' => 'superadmin'
]);

[Q3] What do you think?

[osantelices]

@tabacitu is a beautiful solution on this problem... I've had to use custom fields so far

[thplat]

Hi @tabacitu,

sorry for the late reply.

[Q1] Would you agree? Do you see any other use case I missed?
Yep, you're right. I think selects would be the only things that would come into my mind, too.

Checklist dependencies already handle that behavior, so it's not necessary there in my opinion.

[Q2] I think framing this feature as "scopes" solves that, as you define the entity "the old way", then just reference that one inside the anonymous method. Would you agree?

Although that would be definitely a clean solution, I would not always necessarily return a Builder. Maybe I want to handle custom stuff inside my callback which merges, or performs other operations on the resulting collection. If we made it the "scope" way we could only return a Builder object. But the decision is definitely up to you, I wouldn't implement "callbacks" and "scopes" because both would be doing similar things and might confuse the users.

[Q3] What do you think?

I think this is beautiful. This we we could support callbacks for more "custom" result sets, and would only allow a scope name as string for the scope key. This way we could have the best of both worlds.

[tabacitu]

Awesome! scope it is :-)

[b8ne]

Going to add my 2 cents here, i think adding the feature as a callback is the best method.
Say we have a local scope superAdmin already, we could do this

'scope' => function () {
    return Role::superAdmin()->orderBy('created_at')->get();
}

Or just Role::superAdmin()->get(); if you only want to re-use existing scopes.
Sure scopes should be defined in the model, but forcing the developer to create a scope in the model just for a single field in the controller seems counter intuitive to me.

I have a bit of time free so happy to PR it, let me know.

[mgralikowski]

Thanks for solution. I have similar situation with nested resources (3 lv of nesting).
https://backpackforlaravel.com/articles/tutorials/nested-resources-in-backpack-crud

I need to create select with relationship to parent model. I ended with:

            'callback' => function() use ($formId) {
                return Criterion::where('form_id', $parentFormId)->get();
            }

and i overwritten select.blade.php followed @thplat's proposition.

[tabacitu]

Thanks a lot for the ideas and feedback you guys. I think we finally have a 100% solution, which fixes this issue and a few more, elegantly if I might say so :-)

I made a PR here for it - https://github.com/Laravel-Backpack/CRUD/pull/1694

Let’s move the conversation there please.
Cheers!

[tabacitu]

Please note that despite the fact that the solution has a $query parameter, you can also change the model parameter to use whatever model you want (though I personally don’t see any use case for that).