-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
PHPORM-75 Defer Model::unset($field)
to the save()
#2578
Conversation
Model::unset($field)
to the save()
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #2578 +/- ##
============================================
- Coverage 90.83% 90.64% -0.20%
- Complexity 752 758 +6
============================================
Files 34 34
Lines 1834 1849 +15
============================================
+ Hits 1666 1676 +10
- Misses 168 173 +5
☔ View full report in Codecov by Sentry. |
/** | ||
* @inheritdoc | ||
*/ | ||
public function __call($method, $parameters) |
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.
It's nice to see this gone! :)
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.
I'm not sure why the method unset
was not declared. It might be to allow a relation named unset
.
@@ -488,9 +492,56 @@ public function testUnset(): void | |||
$this->assertTrue(isset($user2->note2)); | |||
|
|||
$user2->unset(['note1', 'note2']); | |||
$user2->save(); |
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.
I believe there was a separate issue where the model was not marked as dirty - maybe assert on isDirty
as well?
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.
It was not dirty since the change was done directly in the database.
I added the assertions on isDirty
for exhautivity.
Fix #2566 and PHPORM-75.
$model->unset($field)
,$model->drop($field)
,unset($model[$field])
,unset($model->field)
must be followed by$model->save()
so they are persisted.$user = User::find($key); unset($user->age); + $user->save();
Deprecate
Model::drop($field)
, it's a duplicate ofModel::unset($field)
. The nativeunset($model->field)
is recommended.Query\Builder::update()
accept a mix of field names and update operators.Todo: