Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A bug with the refresh method (Model class)? #27288

Closed
djug opened this issue Jan 24, 2019 · 6 comments
Closed

A bug with the refresh method (Model class)? #27288

djug opened this issue Jan 24, 2019 · 6 comments

Comments

@djug
Copy link

djug commented Jan 24, 2019

  • Laravel Version: 5.7.13
  • PHP Version: 7.3.1
  • Database Driver & Version: mysql Ver 15.1 Distrib 10.3.12-MariaDB

Description:

in Illuminate/Database/Eloquent/Model.php
if we take a look at the fresh method we find this at the beginning:

if (! $this->exists) {
   return;
}

which mean if the model is no longer there we just return (null?).

but the behavior of the refresh method is slightly different:

if (! $this->exists) {
   return $this;
}

in other terms, if we delete the model, and we execute the fresh() method on it, we will get null, but if we execute refresh() we will get the model back again, which, if I understand correctly, is not the intended goal of this method.

Steps To Reproduce:

>>> $user = User::first()
[!] Aliasing 'User' to 'App\User' for this Tinker session.
=> App\User {#2919
     id: 3,
     name: "Sienna Becker",
     email: "[email protected]",
     email_verified_at: "2018-11-16 06:21:34",
     created_at: "2018-11-16 06:21:34",
     updated_at: "2018-11-16 06:21:34",
   }
>>> $user->delete()
=> true
>>> $user->fresh()
=> null
>>> $user->refresh()
=> App\User {#2919
     id: 3,
     name: "Sienna Becker",
     email: "[email protected]",
     email_verified_at: "2018-11-16 06:21:34",
     created_at: "2018-11-16 06:21:34",
     updated_at: "2018-11-16 06:21:34",
   }
>>> 

if someone could confirm that this is a bug I'll send a MR for it

@mfn
Copy link
Contributor

mfn commented Jan 24, 2019

As evident by gits history, it was made on purpose albeit with no discussion I could quickly find:

Best to ask @themsaid wo did the change.

But in any event, it's seems quite deliberate and not like a bug so a change should be discussed on https://github.com/laravel/ideas

@driesvints
Copy link
Member

@mfn thanks for explaining!

@djug
Copy link
Author

djug commented Jan 24, 2019

@driesvints I think we still need to wait for @themsaid for this one, since he wrote the code (because there is a chance the code needs to be fixed).

could you please reopen it?

@driesvints
Copy link
Member

@djug I pinged him but you can always reach him on discord etc

@devcircus
Copy link
Contributor

These two methods were designed to have different behavior. This is what I would expect.

@themsaid
Copy link
Member

themsaid commented Jan 24, 2019

This was made like this so $model->refresh()->exists return false after deletion. You still have the model object after it was deleted so you can check the current state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants