Laravel-Backpack / community-forum

A workspace to discuss improvement and feature ideas, before they're actually implemented.
28 stars 0 forks source link

[Bug] Route [province-inline-create] not defined. #847

Closed tringuyenduc2903 closed 3 months ago

tringuyenduc2903 commented 4 months ago

Bug report

What I did

Create a field with type relationship

What I expected to happen

Just set the required properties for the field as shown in the documentation

  1. php artisan backpack:crud address/province

in ProvinceCrudController file

    use InlineCreateOperation;
  1. php artisan backpack:crud address/district

in DistrictCrudController Controller file

    use FetchOperation;

    /**
     * Define what happens when the Create operation is loaded.
     *
     * @see https://backpackforlaravel.com/docs/crud-operation-create
     * @return void
     */
    protected function setupCreateOperation()
    {
        ...
        CRUD::addField([
            'type' => 'relationship',
            'name' => 'province_id',
            'ajax' => true,
            'inline_create' => [
                'entity' => 'province',
            ]]);
        ...
    }

    /**
     * Gets items from database and returns to selects.
     *
     * @return JsonResponse|Collection|LengthAwarePaginator|Paginator
     */
    protected function fetchProvince()
    {
        return $this->fetch(Province::class);
    }

in District Model file

    /**
     * @return BelongsTo
     */
    public function province(): BelongsTo
    {
        return $this->belongsTo(Province::class);
    }
  1. php artisan make:migrate create_address_tables
/**
     * Run the migrations.
     */
    public function up(): void
    {
        Schema::create('provinces', function (Blueprint $table) {
            $table->id();
            $table->bigInteger('code')->unique();
            $table->string('name', 40);
            $table->tinyInteger('division_type');
            $table->softDeletes();
        });

        Schema::create('districts', function (Blueprint $table) {
            $table->id();
            $table->bigInteger('code')->unique();
            $table->string('name', 40);
            $table->tinyInteger('division_type');
            $table->foreignId('province_id')
                ->nullable()
                ->constrained()
                ->onDelete('set null')
                ->cascadeOnUpdate();
            $table->softDeletes();
        });
    }

What happened

Exception Occurred: Route [province-inline-create] not defined.

image

What I've already tried to fix it

namespace Backpack\Pro\Http\Controllers\Operations;
...
trait InlineCreateOperation
{
    ...
    protected function setupInlineCreateRoutes($segment, $routeName, $controller)
    {
        Route::post($segment.'/inline/create/modal', [
           \\ From: 'as'        => $segment.'-inline-create',
           \\ To:   'as'        => $routeName.'-inline-create',

           \\ From: 'as'        => $segment.'-inline-create-save',
           \\ To:   'as'        => $routeName.'-inline-create-save',
            ...
        ]);
    }

Is it a bug in the latest version of Backpack?

After I run backpack composer update backpack/crud the error it still persists

Backpack, Laravel, PHP, DB version

When I run php artisan backpack:version the output is:

image

tringuyenduc2903 commented 3 months ago

@karandatwani92 Hello, do you mind my asking questions about the progress of the problem?

My issue is rated as Important, I even point out the error in your source code and how to fix it. But the processing time seems to take longer than expected. So disappointed with the Pro service from Backpack

pxpm commented 3 months ago

Hello @tringuyenduc2903

I don't know why this is categorized as a MUST, as it's not even confirmed it's a bug nor there are any steps to reproduce said error.

We couldn't do something like what you propose in a minor version, as it would be a breaking change.

But I think the error is the fact that you are missing adding the InlineCreateOperation in your secondary CrudController, in your case it seems to be ProvinceCrudController.

That operation should be registered in BOTH controllers as stated in the docs.

There is an extra route registered that's never called (the one you pointed out). That's due to architectural limitations on how we use the Operations. Maybe using some meta-programming we may be able to remove the registration of that extra route, but it's not a priority.

Let me know if you still experience the issue, if so, can you please provide me stripped down controllers of where you are using the operation ?

Sorry it took so much time to get back here.

Cheers

tringuyenduc2903 commented 3 months ago

@pxpm Sorry for being impatient, I've updated the information on how to reproduce the error. If you have difficulty recreating, please comment here. I was really impatient so I couldn't control my words

pxpm commented 3 months ago

Hey @tringuyenduc2903 no issues. This shouldn't be so much time without a response, that's unacceptable I agree with you 100% and sorry for that.

Like I said, I think you are missing adding the InlineCreateOperation in one of your Controllers, in this case DistrictCrudController as you already have it in ProvinceCrudController.

Can you confirm ?

Cheers

tringuyenduc2903 commented 3 months ago

@pxpm

  1. InlineCreateOperation at Address/DistrictCrudController will create router_name "address/district-inline-create". => Not necessary

  2. InlineCreateOperation at Address/ProvinceCrudController will create router_name "address/province-inline-create".

  3. In fact, the field_type relationship needs to get the router_name "province-inline-create", so the exception "Route [province-inline-create] not defined."

Do you understand the problem?

pxpm commented 3 months ago

Ohhhhhh, I think I get the problem now. Thanks for the clear explanation. 🙏

Your segment is address/district and address/province not just /district or /province ?

In that case you can manually set the create and modal urls:

'inline_create' => [
'create_route' => // use here the generated route
'modal_route' => // use here the generated modal route
]

Even this is not a permanent solution, we are aware of some other limitations of the InlineCreate, and we hope to fix them in the next major version, when we can do some Breaking Changes.

Let me know if this solves your problem, at least for now. 👍

Cheers

tringuyenduc2903 commented 3 months ago

@pxpm This error can be fixed in no more than 1 minute. I mentioned it in the article

namespace Backpack\Pro\Http\Controllers\Operations;
...
trait InlineCreateOperation
{
    ...
    protected function setupInlineCreateRoutes($segment, $routeName, $controller)
    {
        Route::post($segment.'/inline/create/modal', [
           \\ From: 'as'        => $segment.'-inline-create',
           \\ To:   'as'        => $routeName.'-inline-create',

           \\ From: 'as'        => $segment.'-inline-create-save',
           \\ To:   'as'        => $routeName.'-inline-create-save',
            ...
        ]);
    }
pxpm commented 3 months ago

Hey @tringuyenduc2903 I understand the fix is not that complicated.

I appreciate you took the time, and have a working solution for it, but we cannot merge it without a Breaking Change, because all the developers using this Operation until now, are expecting the URL to use the $segment, not $routeName.

To do that breaking change, we need to bump the Major version, to bump the major in this package we need to have an upgrade guide, and notify developers about new Major, otherwise they don't get the updates.

We'd rather do all breaking changes at once, when we release a major version for all the packages. Somewhere in a few months this year.

Does my proposed solution work for you ?

Cheers

pxpm commented 3 months ago

Hey @tringuyenduc2903

if you have time to get back here, please let me know if my proposed solution worked for you. In case it doesn't, that is a bug we can fix for sure, but I hope you solved the issue by now.

Let me know. Cheers

tringuyenduc2903 commented 3 months ago

@pxpm I just reviewed the source code, the value of $segment is now province instead of address/province. Boom! So the error is gone

image

pxpm commented 3 months ago

@pxpm I just reviewed the source code, the value of $segment is now province instead of address/province. Boom! So the error is gone

image

By that you mean, you changed your Route::crud() definition right ?

I just want to make sure we are on the same page, like I told you, I have already a PR or two in line for the next major version where I can do some breaking changes, and I am trying to make sure next time I mess with it, I get it right!

Thanks for you time and patience here. I acknowledge you are a very dedicated person, and I really appreciate all your efforts reporting bugs. I understand that sometimes it gets frustrating, but be sure everything you said is taken into consideration.

Cheers

tringuyenduc2903 commented 3 months ago

@pxpm I already know the main cause of the error

Route::crud('address/province', ProvinceCrudController::class);

will generate $segment is address/province and

Route::prefix('address')->group(function () {
    Route::crud('province', ProvinceCrudController::class);
    Route::crud('district', DistrictCrudController::class);
    Route::crud('ward', WardCrudController::class);
});

will create $segment is province

However, editing route_name is still necessary, because it follows the same Route creation principle as ListOperation, CreateOperation, UpdateOperation,...

        Route::get($segment.'/', [
            'as' => $routeName.'.index',
            'uses' => $controller.'@index',
            'operation' => 'list',
        ]);
pxpm commented 3 months ago

Indeed. That's the problem. 👍

I don't think we can use routeName tho. I think the solution to work on all use-cases would need to be some url parsing.

The issue I have identified to fix in next version, I think will fix this too, because they are very similar.

For example, in a nested CRUD, like: https://github.com/Laravel-Backpack/demo/blob/main/app/Http/Controllers/Admin/PetShop/OwnerPetsCrudController.php we have the same issue.

I think if I fix it for nested cruds, it will be fixed for this scenario too.

Thanks, I will try to remember to take this into consideration.

Cheers