Laravel-Backpack / CRUD

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

[Bug] Cannot update data model after 4.1.44 update #3713

Closed tabacitu closed 3 years ago

tabacitu commented 3 years ago

Originally posted by @kitzberger in https://github.com/Laravel-Backpack/CRUD/issues/3705#issuecomment-840412704

Not sure if this is related to this patch: but after upgrading to 4.1.44 we cannot update our data model any longer.

Our scenario is:

With 4.1.44 there's an exception when trying to update the model in the DB: Bildschirmfoto vom 2021-05-13 10-00-21

ziming commented 3 years ago

I'm encountering the same issue after upgrade to 4.1.44 too. (Integrity constraint violation: 1048 Column 'user_id' cannot be null)

For my code, it is this:

[
                'name' => 'user',
                'type' => 'relationship',
                'entity' => 'user',
                'attribute' => 'full_form_description',
                'model' => User::class,
                'placeholder' => 'Search a user',
                'minimum_input_length' => 1,
                'tab' => 'User Filled',
                'attributes' => ['disabled' => 'disabled'],
                'ajax' => true,
            ],

Hope it helps in the investigation. Before 4.1.44 it works fine

pxpm commented 3 years ago

@kitzberger can you let me know how you are setting up the saving process ?

From my understanding you are using model creating event ?

I've setup an user relation (BelongsTo) in the icon model.

public function user() {
        return $this->belongsTo('App\Models\User','user_id'); //user_id is not nullable in database
    }

I've setup the creating event:

public static function boot() {
        parent::boot();

        static::creating(function($item) {
            $item->user_id = backpack_user()->id;
        });
    }

user_id does not show in the form, only name and icon.

It correctly saves in the database:

image

Same for the update process.

@ziming I could not fully test your scenario but I tested with:

        $this->crud->addField([
            'name' => 'user',
            'type' => 'relationship',
            'entity' => 'user',
            'attribute' => 'name',
        ]);

Same result, user_id is properly saved in the database.

Let me know, Pedro

kitzberger commented 3 years ago

Hi @pxpm, yeah, that's exactly how it's set up in our project, can't see any error though ;-/ When I revert that change manually, it's working again: https://github.com/Laravel-Backpack/CRUD/pull/3705/files Ciao, Philipp

pxpm commented 3 years ago

@kitzberger any chance you don't have user_id in your fillable model property?

Best, Pedro

kitzberger commented 3 years ago

@pxpm, fillable isn't set for that model at all:

    // protected $primaryKey = 'id';
    // public $timestamps = false;
    protected $guarded = ['id'];
    // protected $fillable = [];
    // protected $hidden = [];
pxpm commented 3 years ago

I tought the problem might be that, but I'v just tested without $fillable and using $guarded and it still works.

I am concerned that something might be slipping me here. Any chance your project is open-source so I can try to reproduce it there ?

If not, can you share a little bit of the stack trace that lead to the error ?

Any chance that backpack_user()->id is returning null? Because from the query it seems that it is trying to update the user_id column.

Can you try to see the full query with the bindings ?

You can add this snippet in your AppServiceProvider to log all the db calls with the bindings into storage/logs/larave.log

DB::listen(function($query) {
            Log::info(
                $query->sql,
                $query->bindings,
                $query->time
            );
        });

Thanks @kitzberger for the time spent here helping me to debug this.

Best, Pedro

kitzberger commented 3 years ago

@pxpm, unfortunately I cannot publish the source of our application ;-/

But here's the log file you requested. I've stripped it down to the relevant parts:

[2021-05-18 18:59:03] local.INFO: insert into `issues` (`lastname`, `firstname`, `birthday`, `user_id`, `created_at`, `updated_at`) values (?, ?, ?, ?, ?, ?) ["Kitzberger 2001","Philipp","1986-02-08",1,"2021-05-18 18:59:03","2021-05-18 18:59:03"] 

[2021-05-18 18:59:03] local.INFO: select * from `locations` where `locations`.`id` in (?) and `locations`.`deleted_at` is null [null] 
[2021-05-18 18:59:03] local.INFO: update `issues` set `location_id` = ?, `issues`.`updated_at` = ? where `id` = ? [null,"2021-05-18 18:59:03",109] 

[2021-05-18 18:59:03] local.INFO: select * from `users` where `users`.`id` in (?) and `users`.`deleted_at` is null [null] 

[2021-05-18 18:59:03] local.ERROR: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'user_id' cannot be null (SQL: update `issues` set `user_id` = ?, `issues`.`updated_at` = 2021-05-18 18:59:03 where `id` = 109) {"userId":1,"exception":"[object] (Illuminate\\Database\\QueryException(code: 23000): SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'user_id' cannot be null (SQL: update `issues` set `user_id` = ?, `issues`.`updated_at` = 2021-05-18 18:59:03 where `id` = 109) at /var/www/vendor/laravel/framework/src/Illuminate/Database/Connection.php:669)
[stacktrace]
#0 /var/www/vendor/laravel/framework/src/Illuminate/Database/Connection.php(629): Illuminate\\Database\\Connection->runQueryCallback('update `issues`...', Array, Object(Closure))
#1 /var/www/vendor/laravel/framework/src/Illuminate/Database/Connection.php(495): Illuminate\\Database\\Connection->run('update `issues`...', Array, Object(Closure))
#2 /var/www/vendor/laravel/framework/src/Illuminate/Database/Connection.php(428): Illuminate\\Database\\Connection->affectingStatement('update `issues`...', Array)
#3 /var/www/vendor/laravel/framework/src/Illuminate/Database/Query/Builder.php(2736): Illuminate\\Database\\Connection->update('update `issues`...', Array)
#4 /var/www/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php(799): Illuminate\\Database\\Query\\Builder->update(Array)
#5 /var/www/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(746): Illuminate\\Database\\Eloquent\\Builder->update(Array)
#6 /var/www/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(661): Illuminate\\Database\\Eloquent\\Model->performUpdate(Object(Illuminate\\Database\\Eloquent\\Builder))
#7 /var/www/vendor/backpack/crud/src/app/Library/CrudPanel/Traits/Create.php(179): Illuminate\\Database\\Eloquent\\Model->save()
#8 /var/www/vendor/backpack/crud/src/app/Library/CrudPanel/Traits/Create.php(151): Backpack\\CRUD\\app\\Library\\CrudPanel\\CrudPanel->createRelationsForItem(Object(App\\Models\\Issue), Array)
#9 /var/www/vendor/backpack/crud/src/app/Library/CrudPanel/Traits/Create.php(99): Backpack\\CRUD\\app\\Library\\CrudPanel\\CrudPanel->createOneToOneRelations(Object(App\\Models\\Issue), Array)
#10 /var/www/vendor/backpack/crud/src/app/Library/CrudPanel/Traits/Create.php(34): Backpack\\CRUD\\app\\Library\\CrudPanel\\CrudPanel->createRelations(Object(App\\Models\\Issue), Array)
#11 /var/www/vendor/backpack/crud/src/app/Http/Controllers/Operations/CreateOperation.php(79): Backpack\\CRUD\\app\\Library\\CrudPanel\\CrudPanel->create(Array)
#12 /var/www/app/Http/Controllers/Backend/IssueCrudController.php(2103): App\\Http\\Controllers\\Backend\\IssueCrudController->traitStore()
#13 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Controller.php(54): App\\Http\\Controllers\\Backend\\IssueCrudController->store()
#14 /var/www/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(45): Illuminate\\Routing\\Controller->callAction('store', Array)
#15 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Route.php(219): Illuminate\\Routing\\ControllerDispatcher->dispatch(Object(Illuminate\\Routing\\Route), Object(App\\Http\\Controllers\\Backend\\IssueCrudController), 'store')
#16 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Route.php(176): Illuminate\\Routing\\Route->runController()
#17 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Router.php(681): Illuminate\\Routing\\Route->run()
#18 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(130): Illuminate\\Routing\\Router->Illuminate\\Routing\\{closure}(Object(Illuminate\\Http\\Request))
#19 /var/www/vendor/backpack/crud/src/app/Http/Controllers/CrudController.php(43): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#20 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(153): Backpack\\CRUD\\app\\Http\\Controllers\\CrudController->Backpack\\CRUD\\app\\Http\\Controllers\\{closure}(Object(Illuminate\\Http\\Request), Object(Closure))
#21 /var/www/app/Http/Middleware/CheckIfLoggedInBackend.php(39): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#22 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): App\\Http\\Middleware\\CheckIfLoggedInBackend->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#23 /var/www/app/Http/Middleware/Google2FA.php(35): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#24 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): App\\Http\\Middleware\\Google2FA->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#25 /var/www/vendor/spatie/laravel-csp/src/AddCspHeaders.php(13): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#26 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Spatie\\Csp\\AddCspHeaders->handle(Object(Illuminate\\Http\\Request), Object(Closure), 'App\\\\Http\\\\Csp\\\\Ba...')
#27 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Middleware/SubstituteBindings.php(41): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#28 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Routing\\Middleware\\SubstituteBindings->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#29 /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/VerifyCsrfToken.php(78): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#30 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Foundation\\Http\\Middleware\\VerifyCsrfToken->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#31 /var/www/vendor/laravel/framework/src/Illuminate/View/Middleware/ShareErrorsFromSession.php(49): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#32 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\View\\Middleware\\ShareErrorsFromSession->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#33 /var/www/vendor/laravel/framework/src/Illuminate/Session/Middleware/StartSession.php(56): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#34 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Session\\Middleware\\StartSession->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#35 /var/www/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/AddQueuedCookiesToResponse.php(37): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#36 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Cookie\\Middleware\\AddQueuedCookiesToResponse->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#37 /var/www/vendor/laravel/framework/src/Illuminate/Cookie/Middleware/EncryptCookies.php(67): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#38 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Cookie\\Middleware\\EncryptCookies->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#39 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(105): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#40 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Router.php(683): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#41 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Router.php(658): Illuminate\\Routing\\Router->runRouteWithinStack(Object(Illuminate\\Routing\\Route), Object(Illuminate\\Http\\Request))
#42 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Router.php(624): Illuminate\\Routing\\Router->runRoute(Object(Illuminate\\Http\\Request), Object(Illuminate\\Routing\\Route))
#43 /var/www/vendor/laravel/framework/src/Illuminate/Routing/Router.php(613): Illuminate\\Routing\\Router->dispatchToRoute(Object(Illuminate\\Http\\Request))
#44 /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(170): Illuminate\\Routing\\Router->dispatch(Object(Illuminate\\Http\\Request))
#45 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(130): Illuminate\\Foundation\\Http\\Kernel->Illuminate\\Foundation\\Http\\{closure}(Object(Illuminate\\Http\\Request))
#46 /var/www/vendor/barryvdh/laravel-debugbar/src/Middleware/InjectDebugbar.php(67): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#47 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Barryvdh\\Debugbar\\Middleware\\InjectDebugbar->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#48 /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#49 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#50 /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#51 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#52 /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#53 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#54 /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/CheckForMaintenanceMode.php(63): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#55 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Illuminate\\Foundation\\Http\\Middleware\\CheckForMaintenanceMode->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#56 /var/www/vendor/fideloper/proxy/src/TrustProxies.php(57): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#57 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(171): Fideloper\\Proxy\\TrustProxies->handle(Object(Illuminate\\Http\\Request), Object(Closure))
#58 /var/www/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(105): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Object(Illuminate\\Http\\Request))
#59 /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(145): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#60 /var/www/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(110): Illuminate\\Foundation\\Http\\Kernel->sendRequestThroughRouter(Object(Illuminate\\Http\\Request))
#61 /var/www/public/index.php(55): Illuminate\\Foundation\\Http\\Kernel->handle(Object(Illuminate\\Http\\Request))
#62 {main}
GregaUNK commented 3 years ago

I think Iam having the similar issue. When I have field which is disabled it writes empty string to database instead of omitting it from query. Not sure, but maybe this #3643 is what is causing that?

pxpm commented 3 years ago

@GregaUNK I am almost sure that PR is not causing that kind of issues. The only difference there is if the function is not present in model, we don't call it. Otherwise we would call it even if not present and raising an error.

Disabled inputs don't get submitted by the browser, you could try dd() your request to check what is passing after form submission.

@kitzberger From what seems this is what is happening:

My guess is something you are doing is trying to query the database again to update the issue user_id after the creation, when it was already set in the first create query.

That second time you are attempting to update the user_id somewhere in your code is where it's failing.

On my tests I only get the first DB query like you with user_id correctly set.

Let me know, Pedro

GregaUNK commented 3 years ago

@pxpm then it has to be something else... I have many disabled fields. Everything worked till 4.1.43.

Now I have a problem with these fields. If the field is disabled and you save/update the record it overrides value in table with NULL. Any clue what could be wrong? Should I open a new issue about this?

ziming commented 3 years ago

Hmm maybe it will be helpful to share my update method too to give more context

public function update(UpdateRequest $request)
    {
        $this->removeReadOnlyRequestFields();

        // your additional operations before save here
        $redirect_location = $this->traitUpdate();
        // your additional operations after save here
        // use $this->data['entry'] or $this->crud->entry
        return $redirect_location;
    }

/**
     * This method is mainly used by the update method to remove fields that shouldn't be edited.
     */
    private function removeReadOnlyRequestFields()
    {
        $fieldsToRemove = [
            'field_name_one'
        ];

        $this->crud->removeFields($fieldsToRemove);
    }
tabacitu commented 3 years ago

@ziming / @GregaUNK / @kitzberger , maybe one of you can help us track this down the old-fashioned way 😅 By undoing the changes one-by one and seeing if that fixes it. It shouldn't be that time-consuming if there's a clear broken behaviour between .43 and .44. Can any of you guys help us out?

Here's a diff between 4.1.43 and 4.1.44. Let me strip that down even more, functionality changes have only been done in:

Screenshot 2021-05-24 at 15 06 21 Screenshot 2021-05-24 at 15 06 33

And out of those, I think the only ones that could (maybe... somehow) affect this are:

Any chance one of you can undo each of these in your code, one by one, see if that one particular change fixes the problem?

kitzberger commented 3 years ago

@tabacitu, sure. I've done that already, see previous post:

When I revert that change manually, it's working again: https://github.com/Laravel-Backpack/CRUD/pull/3705/files

tabacitu commented 3 years ago

Ah great! @ziming / @GregaUNK , does undoing that fix it for you guys too?

pxpm commented 3 years ago

Hello guys.

So, I could reproduce @GregaUNK and @ziming issues, but I dunnot how it worked before and now it's not working.

The problem about the disabled fields is the fact that the disabled fields are not sent in the form, while read-only does send the fields.

I tracked it down to where we get the form data: https://github.com/Laravel-Backpack/CRUD/blob/826c009bc47cf522f01a4e6a845caff64c9d2a2e/src/app/Library/CrudPanel/Traits/Create.php#L217

After we get all the fields that are relations, later in: https://github.com/Laravel-Backpack/CRUD/blob/826c009bc47cf522f01a4e6a845caff64c9d2a2e/src/app/Library/CrudPanel/Traits/Create.php#L174 we try to relate the values. Value is null, so we set the value to null in database, like if you have a regular select and remove the previous selected item. But in your case there is no value at all, so we should not touch that relations.

I've created #3747 that will fix this issue for single selects, but not totally because of the multiple ones, more info in the PR thread, but I am still not able to find out any change from .43 => .44 that implicates the issue with disabled inputs.

About @kitzberger issue I am still not able to find out how they could related, because the change was in syncPivot and a belongs to relation does not have pivot => true, unless developer specified it, so all other conditions would be false too.

I will keep digging on this.

Best, Pedro

tabacitu commented 3 years ago

Sorry to be so late to the party, guys! I've been in caveman-mode working on the next big thing for Backpack. I'm pretty sure you'll love it too, super-excited to get it out the door by the end of the month.

@pxpm if the problem is https://github.com/Laravel-Backpack/CRUD/pull/3705 I propose:

Sounds like the fastest & safest solution. Let's talk a bit in https://github.com/Laravel-Backpack/CRUD/pull/3705 please 🙏 I need your experience on this to make this judgement call.

tabacitu commented 3 years ago

Hi guys,

First of all thanks lot for the patience (and for pitching in of course). We've decided to:

Again, thanks a lot for the help. And sorry it took quite a while. Please let us know if you have any more issues with this. Hopefully you won't 😅 🤞

Cheers!