Laravel-Backpack / CRUD

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

Add workaround for quick button inconsistency #5422

Closed pxpm closed 7 months ago

pxpm commented 7 months ago

WHY

BEFORE - What was wrong? What was happening before this PR?

Reported in #5421

Following a simple tutorial that should "just work", was not "just working" 🤷‍♂️

I've debugged the issue and the culprit is the fact that we ->studly($buttonName) to get the access string. So a button with the name comment would have the access string Comment.

AFTER - What is happening after this PR?

It checks both operation name strings. First the regular one and as a fallback the camelCased version of the operation.

HOW

How did you achieve that, in technical terms?

I could have just changed the stub and define the operation button with access => Str::studly($operationName) or just removed the studly() in the quick.blade.php file, but that would be a BC at the moment in any of those cases. But it's what I've planned for v7.

So I made it fallback to a camelCased version of the name as last resort to try to find access. I don't think this would ever be a BC the way I implemented because it wouldn't be possible for you to have a moderate and a Moderate operations, or a SuperModerate and superModerate operations at the same time.

Is it a breaking change?

I don't think so, no.

How can we test the before & after?

Follow the tutorial in Backpack website https://backpackforlaravel.com/docs/6.x/crud-operations#creating-a-new-operation-with-a-form-1 . It will fail previously, it will work after.

pxpm commented 7 months ago

After our talk we decided to do this change on the blade file itself so it only concerns the quick button.

It's better to keep hasAccess() clean from string manipulations. Whatever string it receives, is the string it checks if there is access to. We would risk introducing security issues if we tried to guess access for operations at this level, so we should be better with the solution for quick button in the blade file..

pxpm commented 7 months ago

Also updated docs here: https://github.com/Laravel-Backpack/docs/pull/541