platform icon indicating copy to clipboard operation
platform copied to clipboard

replace diffAssoc with diff so the method is removed when there are parameters in front of it

Open TomNealBorneman opened this issue 2 years ago • 3 comments

Fixes #2216

TomNealBorneman avatar Jul 06 '22 14:07 TomNealBorneman

Hi @TomNealBorneman, could you add some tests to test this case?

tabuna avatar Jul 07 '22 11:07 tabuna

When using Model bindings in your screen method (say the save function for example) you get a 404 when trying to save a new instance of a child relation:

image

image

This happens because it tries to use the method as a key for finding the model. And a model with the key 'save' does not exist. Changing the diffAssoc to diff makes it so the method is removed from the parameters regardless of its position within the collection. The following dumps (made after src\Screen\Screen.php:220) show the different results.

diff: image

diffAssoc: image

Dump: image

This still does not work when you use extra url parts like http://laravel-cms.test/dashboard/users/38/test/create because then it will still use create as a key for the model binding. But in its current iteration the binding does not work at all.

This fix could cause issues if you have models with string keys that could overlap with methods, say if you have a model with the key save it would bind that instead of creating a new model as intended. But this seems like an edge case to me.

This has no effect on saving existing models like so: http://laravel-cms.test/dashboard/users/38/test/1 because the identifier is present and can be used for the binding.

edit: add clarification of parameters dumped in screenshots.

TomNealBorneman avatar Aug 19 '22 09:08 TomNealBorneman

Changing to diff fixes the issue for me :). If this isn't going to get merged, then please provide anther solution.

madyanmalfi avatar May 20 '23 21:05 madyanmalfi