Skip to content

[5.1] Add Model::saveOrFail() to execute save operation within database transaction. #11202

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 7, 2015
Merged

[5.1] Add Model::saveOrFail() to execute save operation within database transaction. #11202

merged 2 commits into from
Dec 7, 2015

Conversation

crynobone
Copy link
Member

This would simplify the following operation:

try {
    DB::transaction(function () use ($device) {
        $device->save();
    });
} catch (Exception $e) {
    return false;
}

to

try {
    $device->saveOrFail();
} catch (Exception $e) {
    return false;
}

Signed-off-by: crynobone crynobone@gmail.com

GrahamCampbell
GrahamCampbell reviewed Dec 5, 2015
View reviewed changes
src/Illuminate/Database/Eloquent/Model.php
$saved = false;

$this->getConnection()->transaction(function () {
$saved = $this->save($options);
Copy link
Member

Choose a reason for hiding this comment

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

this function will not modify the saved variable - php puts it in a different scope

@GrahamCampbell
Copy link
Member

Needs tests because the current implementation doesn't actually work.

@crynobone
[5.1] Add Model::saveOrFail() to execute save operation within databa…
3496a5a 
…se transaction.

Signed-off-by: crynobone <crynobone@gmail.com>
@crynobone crynobone force-pushed the feature/save-or-fail branch from 06fcda3 to 3496a5a Compare December 5, 2015 11:57
crynobone
crynobone reviewed Dec 5, 2015
View reviewed changes
tests/Database/DatabaseEloquentIntegrationTest.php
$user = EloquentTestUser::first();
$this->assertEquals('otwell@laravel.com', $user->email);
$this->assertEquals(1, $user->id);
});
Copy link
Member Author

Choose a reason for hiding this comment

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

This tests follow testNestedTransactions() with the only different with using saveOrFail().

@crynobone
Copy link
Member Author

Needs tests because the current implementation doesn't actually work.

Duplicate Illuminate\Database\Connection::transaction(), rebase and add tests.

GrahamCampbell
GrahamCampbell reviewed Dec 5, 2015
View reviewed changes
src/Illuminate/Database/Eloquent/Model.php
$db->rollBack();

throw $e;
} catch (Throwable $e) {
Copy link
Member

Choose a reason for hiding this comment

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

no need for all this, just use finally

Copy link
Member

Choose a reason for hiding this comment

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

oh, wait, ignore me - we don't want to always rollback, lol

@crynobone
Add missing import.
777efcd 
Signed-off-by: crynobone <crynobone@gmail.com>
@crynobone crynobone force-pushed the feature/save-or-fail branch from 0da75d2 to 777efcd Compare December 5, 2015 12:22
@taylorotwell taylorotwell merged commit 777efcd into laravel:5.1 Dec 7, 2015
@crynobone crynobone deleted the feature/save-or-fail branch April 19, 2017 00:20
@caugner caugner mentioned this pull request Aug 30, 2021
Merged
@fripig fripig mentioned this pull request Sep 1, 2021
Open
@caugner caugner mentioned this pull request Sep 13, 2021
Merged
@peterfox peterfox mentioned this pull request Sep 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@crynobone @GrahamCampbell @taylorotwell