[Feature Request] link() helper on CrudColumn

[karandatwani92]

WHY

Solves Laravel-Backpack/community-forum#8

Context

@karandatwani92 let's do a prototype for:

CRUD::column('category')->linkTo('category.show'); // note that this is the ROUTE NAME, not OPERATION NAME

We might also add a helper (macro) for ease of use:

CRUD::column('category')->linkToShow(); that points to route('monster.show', $id) if that route exists.

But not in the prototype. Let's remember to also test this on morph relationships.

How to use

CRUD::column('select')
->type('select')
->entity('category')
->attribute('name')
->linkTo('category.show');// OR ->linkTo('category.show','_blank')

CRUD::column('text')->linkTo('https://www.google.com/search?q=');
CRUD::column('sku')->linkTo('https://www.bulkmake.com/p/');

[tabacitu]

Hey @karandatwani92 ,

I could NOT get this to work 😔

Try 1. In our demo, tried in OwnerCrudController, throws an array to string conversion error.

        CRUD::column('owner')->linkTo('owner.show');

Try 2. In our demo, tried in ArticleCrudController, tried this but all links point to the same entry, with the ID of 1:

            $this->crud->addColumn([
                'label' => 'Category',
                'type' => 'select',
                'name' => 'category_id',
                'entity' => 'category',
                'attribute' => 'name',
            ]);
            $this->crud->column('category_id')->linkTo('category.show');

Try 3. In our demo, tried in CategoryCrudController to do the below, but I get the same array to string conversion error:

        CRUD::column('parent')->linkTo('category.show');

Try 4. In our demo, tried in CategoryCrudController to do the below and I get the same array to string conversion error:

        CRUD::column('articles')->linkTo('article.show');

---

So... no matter what I try, this doesn't work as expected for me. What's happening here? Am I using this wrong? AFAIK, I'm using it exactly like in the PR instructions.

Let me re-define my expectations here:

• I expect to be able to do ->linkTo('entity.show') on any column... and for it to add a link to the show operation, towards that particular connected entry; click to go see that related entry;
• I expect to be able to use this linkTo() helper on any Backpack column type that has a relationship, in CRUD or PRO;
• I expect to be able to use this linkTo() helper on both single-relationship columns, and multiple-relationship columns;

What's gone wrong here? Are my expectations too high? Does the PR only treat one particular use case, that I haven't tested?

You want to try a re-do on this PR?

Cheers!

[tabacitu]

Decided to postpone this to next month, in order for @karandatwani92 to be able to finish this. @karandatwani92 when you're back, please give my review above a look, fix things and talk to Antonio & Pedro.

[tabacitu]

@pxpm please take over and let's get this past the finish line this month 💪

[pxpm]

@tabacitu docs can be found here: Laravel-Backpack/docs#519

Please do the final review and let me know if we have the 🚥 here.

Cheers

[susanu]

Hi guys,

I like this new feature, but, how do we handle links such as index with filters?
Example:

$this->crud->addColumn([
    'label'     => trans('backpack::permissionmanager.users'),
    'type'      => 'text',
    'name'      => 'users_count',
    'wrapper'   => [
        'href' => function ($crud, $column, $entry, $related_key) {
            return backpack_url('user?role='.$entry->getKey());
        },
    ],
    'suffix'    => ' '.strtolower(trans('backpack::permissionmanager.users')),
]);

[susanu]

Hi @tabacitu,

In my code i have an article crud and a category crud.
After i tested i found the following errors:
When i use it on a relationship_count column.

CRUD::column('articles')
	->type('relationship_count')
	->linkTo('admin.article.show');

Method Backpack\CRUD\app\Library\CrudPanel\CrudPanel::isAttributeInRelationString does not exist.

When i use it on a relationship column.

CRUD::column('category')
    ->type('relationship')
    ->linkTo('admin.category.show');

Undefined property: Backpack\CRUD\app\Library\CrudPanel\CrudColumn::$name

I'll stick with my implementation, keep it simple and let laravel throw errors if the route is not configured properly.

if (!CrudColumn::hasMacro('linkTo')) {
    CrudColumn::macro('linkTo', function (string|Closure $route, ?string $target = null): static {
        /** @var CrudColumn $this */
        $wrapper = $this->attributes['wrapper'] ?? [];

        $wrapper['href'] = function ($crud, $column, $entry, $relatedKey) use ($route) {
            if (is_callable($route)) {
                return $route($entry, $relatedKey);
            }

            return route($route, $relatedKey);
        };

        if ($target) {
            $wrapper['target'] = $target;
        }

        return $this->wrapper($wrapper);
    });
}

Configuration:

CRUD::column('articles')
	->type('relationship_count')
	->linkTo(fn($entry, $related_key) => route('admin.article.index', ['category_id' => $entry->getKey()]));

CRUD::column('category')
    ->type('relationship')
    ->linkTo('admin.category.show');

[pxpm]

Hey @susanu thanks for the suggestions.

I agree with the use case you presented, re-activating filters on list pages, but there are others that we are consciously leaving behind, like nested cruds that require 2 parameters. I've categorized those a advanced usecases and that's why I didn't cover them.
But let's explore some real scenarios here:

1 - I want to redirect a user to a List Page with some filter pre-activated that does not depend on $entry:

CRUD::column('article')->linkTo('articles.list', ['filterName' => 'filterValue']);

// VS

CRUD::column('article')->wrapper(['href' => backpack_url('articles?filterName=filterValue')]);

2 - I want to redirect a user to a List Page with some filters pre-activated that depend on $entry:

CRUD::column('article')->linkTo('articles.list', [
    'filterName' => function($crud, $column, $entry, $related_key) {
        return $entry->someAttribute;
    },
    'otherFilter' => function($crud, $column, $entry, $related_key) {
        return $entry->otherAttribute;
    },
]);

// VS

CRUD::column('article')->wrapper(['href',  function($crud, $column, $entry, $related_key) {
        return backpack_url('articles?filterName='.$entry->someAttribute.'&otherName='.$entry->otherAttribute); // or a sprintf or whatever your flavor.
    }]);

//So what about this:
CRUD::column('article')
    ->linkTo('articles.list', [
        LinkParameter::entry('filterName', fn($entry) => $entry->someAttribute),
        LinkParameter::related('id'), // we know it's to use the `$related_key` in a param named `id`
        LinkParameter::for('otherName', fn($crud, $column, $entry, $related_key) => $column['smt'] ? $entry->bla : $entry->ble),
    ])
);

I don't see the point for the linkTo() in this advanced usecases if we don't completely over-haul the implementation to have a "nicer" DX.

So to answer @tabacitu my responses to both questions are **Yay** as long as we finish this DX and the CrudControllers don't become a messyguetti (more 😈 ).

I think @susanu may have some CRUD/Library files overridden from the error he's getting:

Method Backpack\CRUD\app\Library\CrudPanel\CrudPanel::isAttributeInRelationString does not exist.

CRUD/src/app/Library/CrudPanel/Traits/Relationships.php

Line 364 in 0bd0b2a

public function isAttributeInRelationString(array $field): bool

There is a reason for that to be there, otherwise not all the columns will work as you can see from what the code is doing.

I will test the columns that you guys said are failing as that's independent from the DX.

Cheers

[susanu]

Hi @pxpm,

Regarding the error Method Backpack\CRUD\app\Library\CrudPanel\CrudPanel::isAttributeInRelationString does not exist., my bad there, i tested copying only the macro.

Try using this implementation:

if (!CrudColumn::hasMacro('linkTo')) {
    CrudColumn::macro('linkTo', function (string|Closure $link, ?string $target = null): static {
        /** @var CrudColumn $this */
        $wrapper = $this->attributes['wrapper'] ?? [];

        $wrapper['href'] = function ($crud, $column, $entry, $relatedKey) use ($link) {
            if (is_callable($link)) {
                return $link($entry, $relatedKey);
            }

            $route = Route::getRoutes()->getByName($link);

            if ($route) {
                return route($link, $relatedKey);
            }

            return $link;
        };

        if ($target) {
            $wrapper['target'] = $target;
        }

        return $this->wrapper($wrapper);
    });
}

This covers most of the use cases, i think:

CRUD::column('article')->linkTo(route('articles.list', ['filterName' => 'filterValue']));
CRUD::column('article')->linkTo(backpack_url('articles?filterName=filterValue'));
CRUD::column('article')->linkTo(fn($entry, $related_key) => route('articles.list', ['filterName' => $entry->getKey()]));
CRUD::column('article')->linkTo(fn($entry, $related_key) => 'https://external-url.com/article/preview/' . $entry->getKey());
CRUD::column('article')->linkTo('https://google.com/', '_blank');
CRUD::column('article')->linkTo('articles.list');

Also, i love this

//So what about this:
CRUD::column('article')
    ->linkTo('articles.list', [
        LinkParameter::entry('filterName', fn($entry) => $entry->someAttribute),
        LinkParameter::related('id'), // we know it's to use the `$related_key` in a param named `id`
        LinkParameter::for('otherName', fn($crud, $column, $entry, $related_key) => $column['smt'] ? $entry->bla : $entry->ble),
    ])
);

[tabacitu]

I like the how LinkParameter looks too, but @pxpm let's please not do that. If we do that, it's a clear sign that it has become too complex. This is supposed to be a simple feature that helps you add links in the most common scenarios. If we have to add a Class for it, it's already too complex, we have wrapper for those, and you can do whatever you want there. So... no go on that one, please 😀

Hmmm I like this too, seems like a simple solution to a complex problem:

CRUD::column('article')->linkTo(route('articles.list', ['filterName' => 'filterValue']));
CRUD::column('article')->linkTo(backpack_url('articles?filterName=filterValue'));
CRUD::column('article')->linkTo(fn($entry, $related_key) => route('articles.list', ['filterName' => $entry->getKey()]));
CRUD::column('article')->linkTo(fn($entry, $related_key) => 'https://external-url.com/article/preview/' . $entry->getKey());
CRUD::column('article')->linkTo('https://google.com/', '_blank');
CRUD::column('article')->linkTo('articles.list');

But what I don't see is how it would cover the link to the Show operation, where we would need to inject the ID of the current entry. Would that be covered by CRUD::column('article')->linkTo('articles.show'); just as it is now?

So what does the parameter for this function become? Docs:

• Closure that will return the full URL to redirect to;
• string to route name;
• string to URL;

The only thing I don't like here is that string can be both a URL and a route name... but I guess I could live with that if it adds more customizability... What do you think @pxpm ? Is this a change for the good or over-doing it?

[susanu]

Would that be covered by CRUD::column('article')->linkTo('articles.show'); just as it is now?

Yes

My humble opinion is that we can live with the first parameter being a wildcard instead of having 2 options for defining a url (linkTo and wrapper).

[pxpm]

The suggested code by @susanu wouldn't work, but the idea might work yes.
Let's see the following routes:

Route::get('article/{article}/publish')->name('article.publish'); 
// http://demo.test/admin/article/1/publish

Route::get('article/{id}/show')->name('article.show') 
// http://demo.test/admin/article/1/show

Route::get('article')->name('article.list')
 // http://demo.test/admin/article

If you defined: CRUD::column('article')->linkTo('article.publish') or CRUD::column('article')->linkTo('article.show') we could infer that the route require 1 parameter and the developer did not provide any, so we will use the $related_key in that parameter. (as we are doing now).

But what if you want to add an additional url parameter when publishing/showing an article eg: article/1/publish?mode=someEntryValue ?
The developer just need to add the additional parameters and we would still infer the related parameter if not present. So:

CRUD::column('article')
    ->linkTo('article.publish', [
        'article' => fn($entry, $related_key) => $related_key, 
        'mode' => fn($entry) =>$entry->someAttribute`
    ]);

// is the same as:
CRUD::column('article')
    ->linkTo('article.publish', [ 'mode' => fn($entry) =>$entry->someAttribute`]);
 // the $related_key parameter (article) will be automatically added since it's missing in the parameter list

To sum up the rules:

linkTo('route.name')

• If route has no parameters just return route('route.name').
• If the route has 1 parameter return route('route.name', [parameterName => $related_key])
• Abort if the the route does not exist, or require more than one parameter.

linkTo('route.name', ['starting_at' => fn($entry) => $entry->created_at])

• If route has no parameters just return route('route.name', ['starting_at' => '2023-10-24'])
• If route has one parameters that's not starting_at, say /{id}/ we will return route('route.name', ['id' => $related_key, 'starting_at' => '2023-10-24'])
• Abort if the route does not exist or require more parameters than the ones provided + $related_key parameter.

linkTo(fn($entry) => route('route.name', ['id' => $entry->id]))

• just return the closure value

Are we on-board with this ?

[susanu]

Are we on-board with this ?

Yes, looks good to me.

[pxpm]

@tabacitu did the changes we discussed here.

Added tests for all the scenarios I could remember, do you mind testing the scenarios you found failing or see if I miss them in the tests ?

@susanu anything you think I didn't covered or you would do differently ?

Updated docs are here: Laravel-Backpack/docs#519

Cheers

[tabacitu]

Woohooo! Excellent work here @pxpm . I think you really knocked it out of the park!

I tested this in multiple scenarios and found no problems. It works exactly as I expect it to. Excellent breakdown in the comment above too 👏👏👏

[tabacitu]

This is ready to go from my POV, take it away @pxpm / @promatik ! 🎉