Crud: Previous CrudController edit method functionality is now broken

[bauersfeld]

Bug report

Previous functionality is now broken as a results of fixes #1323 and #252. I was relying on the CrudController edit method as a singleton to edit specific User records by id from within my application (with a custom route to call edit with no id). But now you have some new logic that I don't understand that looks to the route for the id and takes precedence over the the method $id parameter making the $id parameter useless. Any recommendation on how to work with this new logic? I have a hack in place below. Thx

What I did:

Custom Route:

Route::get('profile', 'UserCrudController@profile');

UserCrudController:

public function profile() {
parent::edit(Auth::user()->id);
}

What I expected to happen:

Display the edit view for the specific user id

What happened:

No query results for model [App\Models\User]

What I've already tried to fix it:

To get around the new logic, I establish the entry property before making the call

public function profile() {
$this->entry = $this->crud->getEntry(Auth::user()->id); // <= HACK
parent::edit(Auth::user()->id);
}

Backpack, Laravel, PHP, DB version:

[welcome[bot]]

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps _a lot_ in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication mediums:

• Bug Reports, Feature Requests - Github Issues (here);
• Quick help (_How do I do X_) - Gitter Chatroom;
• Long questions (_I have done X and Y and it won't do Z wtf_) - Stackoverflow, using the backpack-for-laravel tag;

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome _awesome_ community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

--
Justin Case
The Backpack Robot

[iMokhles]

which Backpack/Crud version you have ?

[bauersfeld]

The latest version, 3.4.5

[iMokhles]

if you have used latest version ( i have tested it myself now for another Crud table )

return parent::edit(2); works well

and yes for latest version you need to use backpack_user()->id rather than Auth::user()->id because latest version uses custom guard name web if not defined in config/backpack/base.php => guard

[bauersfeld]

I have a fresh install of laravel (5.6.17) and laravel-backpack crud (3.4.5). Here is my stripped down controller that still has the problem. Appreciate the help.

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 Backpack\PermissionManager\app\Http\Requests\UserStoreCrudRequest as StoreRequest;
use Backpack\PermissionManager\app\Http\Requests\UserUpdateCrudRequest as UpdateRequest;

class UserCrudController extends CrudController
{
    public function setup()
    {
        $this->crud->setModel('App\User');
        $this->crud->setRoute(config('backpack.base.route_prefix') . '/user');
        $this->crud->setEntityNameStrings('user', 'users');
        $this->crud->setFromDb();
    }

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

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

    public function profile()
    {
        $id = backpack_user()->id;
        // $this->entry = $this->crud->getEntry($id);
        return parent::edit($id);
    }
}

[bauersfeld]

The key thing is the route, it's doesn't have the record id in it

http://localhost:8000/admin/profile

[iMokhles]

mmmm you main issue was ( No query results for model [App\Models\User] )
and your crud model is ( $this->crud->setModel('App\User'); )
so there are a complete different here between models ( which one you use )
and about $id = backpack_user()->id; it get's the current authenticated ( Admin ) id

[bauersfeld]

I apologize, my original example had the model at App\Models\User from my actual app, but then I created a fresh separate app to isolate the problem and left the User model in its default location of App\User. If you wouldn't mind trying a quick test on your code that tested the problem. Create custom route to a method with no params that calls the parent edit method with a constant value and try to access the route from the browser. I seem to be able to reproduce the issue easily. Thx

Route::get('profile', 'UserCrudController@profile'); // No id parameter

public function profile() {  // No id parameter
     parent::edit(2);   // Call parent edit method
}

[AbbyJanke]

I see where the issue is and the simple way to fix it.

Backpack/CRUD/CrudRouter.php:

        Route::get($this->name.'/{id}/details', [
            'as' => 'crud.'.$this->name.'.showDetailsRow',
            'uses' => $this->controller.'@showDetailsRow',
        ]);

        Route::get($this->name.'/{id}/translate/{lang}', [
            'as' => 'crud.'.$this->name.'.translateItem',
            'uses' => $this->controller.'@translateItem',
        ]);

        Route::get($this->name.'/{id}/revisions', [
            'as' => 'crud.'.$this->name.'.listRevisions',
            'uses' => $this->controller.'@listRevisions',
        ]);

        Route::post($this->name.'/{id}/revisions/{revisionId}/restore', [
            'as' => 'crud.'.$this->name.'.restoreRevision',
            'uses' => $this->controller.'@restoreRevision',
        ]);

We would simply need to add to change {id} to {id?}`

In the CrudController edit() change this line:

$id = $this->crud->getCurrentEntryId() ?? $id;

to something more like

if(is_null($id))
{
$id = $this->crud->getCurrentEntryId() ?? $id;
}

Reason to keep it is a fallback incase no record ID gets passed through from the child.

For now my suggestion @bauersfeld would be to create a custom 'CrudController' and extend that instead of the one included with the package.

[bauersfeld]

I ended up doing something similar. Thanks for followup.

[iMokhles]

You was trying to access user's profile ( which user model you need) CRUD's user is the Admin Model not User model specally in the last versions so trying to get Admin id and passing it as variable to ( edit function of another Model CRUD Controller) it won't work unless you have already user with the same id otherwise it won't work at all

[joostjacobs]

Whoever made those changes to getCurrentEntryId() also failed to check if this has any adverse effects on the Revisions functionality of backpack.

If you have 10 records, and you edit the first record 10 times and then undo the latest edit using the revisions feature, $id will become 10, therefore _applying the change to the wrong entry_. The getCurrentEntryId() is using the route to see what the id should be. And it is looking at the last one in the route. But in the revision routes, the last ID is the revisionID, not the entry id.

So if we take this as an example:

/company/1/revisions/12/restore 

Then id will become 12 as a result of those modifications, while it should be 1.

That being said, it is strange to me that the restoreRevision() on the Revisions trait is trying to resolve all those id's: they are already in the route, so why not simply accept them both as a parameter?

e.g. change

public function restoreRevision($id)

to

public function restoreRevision($id, $revisionId)

And then stripping the getCurrentEntryId() and \Request::input() for $id and $revisionId respectively.

[tabacitu]

@joostjacobs you're right, thank you for the PR that fixes it!

@bauersfeld @AbbyJanke @iMokhles glad you got to the bottom of this - will close it then, as it looks like a niche use case to me.

Cheers!