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

[5.4] Add Model::refresh() #19174

Merged
merged 3 commits into from
May 15, 2017
Merged

[5.4] Add Model::refresh() #19174

merged 3 commits into from
May 15, 2017

Conversation

themsaid
Copy link
Member

@themsaid themsaid commented May 12, 2017

This method eliminates the need for:

$user = $user->fresh()

You can do this instead:

$user->refresh()

@themsaid themsaid changed the title [5.4] Add model refresh [5.4] Add Model::refresh() May 12, 2017
@mattstauffer
Copy link
Contributor

Woo!

@calebporzio
Copy link
Contributor

Wondering if there is a way to not have to call ->load(... every time I want to refresh(). Like ->fresh() has optional "with" param... possibly same deal here?

@themsaid
Copy link
Member Author

refresh() will reload all relationships currently loaded automatically.

@calebporzio
Copy link
Contributor

got it, cool!

@morloderex
Copy link
Contributor

@themsaid what is the different between fresh and refresh here?

couldn't refresh just call fresh?

@decadence
Copy link
Contributor

decadence commented May 13, 2017

@morloderex fresh loads relations based on $with parameter, refresh reloads all relations based on current model loaded relations.

@@ -944,6 +944,23 @@ public function fresh($with = [])
}

/**
* Reload the current model instance with fresh attributes from the database.
*
* @param array|string $with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No param


$this->load(array_keys($this->relations));

$this->setRawAttributes(static::find($this->getKey())->attributes);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be findOrFail? What should happen if the model has been deleted in the meantime?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the model was deleted exists would return false and the method will just bail.

@ghost
Copy link

ghost commented May 15, 2017

Where are unit tests?

@JosephSilber
Copy link
Member

JosephSilber commented May 15, 2017 via email

@themsaid
Copy link
Member Author

@JosephSilber good point, switched to using findOrFail

@taylorotwell taylorotwell merged commit ce694c3 into laravel:5.4 May 15, 2017
@projct1
Copy link

projct1 commented May 31, 2017

@themsaid Why refresh return a void, not the model itself? Its inconvenient for use in chains:

return view('admin.selections.list-item')->with('selection', $selection->refresh());

@themsaid
Copy link
Member Author

@rorc you can use fresh() for that.

@projct1
Copy link

projct1 commented May 31, 2017

@themsaid But, if i want reload only current relations?

@themsaid
Copy link
Member Author

@rorc just call refresh() and return the original value of yours, thats' the point :)

@projct1
Copy link

projct1 commented May 31, 2017

@themsaid Sigh, except 1 line i have to use 2 lines? Are you kidding? 😒

$selection->refresh();

return view('admin.selections.list-item')->with('selection', $selection);

@themsaid
Copy link
Member Author

@rorc haha .. well I think I have an idea

@projct1
Copy link

projct1 commented May 31, 2017

@themsaid Why not just return the refreshed model? 😉

@Vasiliy-Bondarenko
Copy link

plus 1 vote for
return $this;
instead of void.

Very useful for chaining in tests.

        $comment->refresh(); // redundant line here!
        $this->assertCount(1, $comment->replies);

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

Successfully merging this pull request may close these issues.

10 participants