Laravel-Backpack / CRUD

Build custom admin panels. Fast!
https://backpackforlaravel.com
MIT License
3.04k stars 885 forks source link

[Bug] `setAccessCondition()`causing error on create operations #5410

Closed hagealex closed 7 months ago

hagealex commented 8 months ago

Bug report

What I did

I wanted to make us of the new setAccessCondition feature for my CRUD controllers. To test it I added this code into the setup() method of one of my controllers:

$this->crud->setAccessCondition(['update', 'delete'], function ($entry) {
    return $entry->id==1 ? true : false;});

What I expected to happen

I expect that the callback is only run for the specific operations.

What happened

When performing a create action I get this error: grafik grafik

I didn't expect the callback method to be run here because obviously, during the create operation, there is no $entry.

What I've already tried to fix it

??

Is it a bug in the latest version of Backpack?

Yes

After I run composer update backpack/crud the bug... is it still there?

Yes

Backpack, Laravel, PHP, DB version

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

### LARAVEL VERSION:
10.32.1.0

### BACKPACK PACKAGE VERSIONS:
backpack/basset: 1.2.1
backpack/crud: 6.4.2
backpack/generators: v4.0.2
backpack/permissionmanager: 7.1.1
backpack/pro: 2.0.18
backpack/theme-coreuiv4: 1.1.1
welcome[bot] commented 8 months ago

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 channels:

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

karandatwani92 commented 7 months ago

Hey @hagealex

The closure function is expected to run/check in every operation, helping us to hide the buttons from the current operation.

Notice that we are on list operation but controlling other operation's access+buttons(update & delete).

The first param is to specify which operation's access will be modified and this is working fine.

BUT you are right too! The documented code is expected to run flawlessly.

Hey @pxpm please check what's wrong here https://backpackforlaravel.com/docs/6.x/crud-operations#handling-access-to-operations

I tried to escape the error, but none worked!

pxpm commented 7 months ago

Hey @hagealex thanks for the report.

I think the docs are misleading in this example, since what you are telling CRUD is that: run this access closure on EVERY operation, but actually you want to run it in the list operation (when you have entries in the table) or check if there is entry or not in the closure.

There is no $entry on every operation as you said, and Backpack will return null when no entry is present, in that case should be the developer the one to check if he wants to allow something with/without an entry.

We can either change the closure to deal with empty entries: is_null($entry) ? true : ($entry->id === 1 ? true : false) or add that access closure only to the list operation:

$this->crud->operation('list', function() {
        $this->crud->setAccessCondition(['update', 'delete'], function ($entry) {
            return $entry->id==1 ? true : false;
    });
});

I've added this example to the docs, as I think it's the most appropriate scenario. Let me know if you have any doubts or the example is not clear enough.

Thanks @karandatwani92 for bringing this to my attention 🙏

Cheers 🥂

hagealex commented 7 months ago

@pxpm and @karandatwani92 thank you for your support here! Now it's much more clear for me how to use this feature the right way!