-
Notifications
You must be signed in to change notification settings - Fork 1.4k
PHPORM-171 Set timestamps when using createOrFirst
#2905
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
Conversation
createOrFirst
|
||
/* @see \Illuminate\Database\Eloquent\Model::performInsert */ | ||
if ($instance->usesTimestamps()) { | ||
$instance->updateTimestamps(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c5bedea
to
ed949b9
Compare
$user1 = User::createOrFirst(['email' => '[email protected]']); | ||
|
||
$this->assertSame('[email protected]', $user1->email); | ||
$this->assertNull($user1->name); | ||
$this->assertTrue($user1->wasRecentlyCreated); | ||
$this->assertEquals($createdAt, $user1->created_at->getTimestamp()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use a comparison to assert that the created_at
and updated_at
fields are greater than or equal to the previously calculated current time?
I don't see anything in the PR that would guarantee the entire test completes within a single timestamp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Carbon::setTestNow
freezes the clock, so that the date point that is used to populate create_at
is the one that is set.
Fix PHPORM-171
Issue reported by @nicohell #2720 (comment)
Checklist