-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Allow to sync a single model instance #2396
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
Fix `sync()` method not handling properly single model's instances passed via `$ids` argument
Issue #2395
Codecov Report
@@ Coverage Diff @@ ## master #2396 +/- ## ============================================ - Coverage 87.62% 87.57% -0.05% - Complexity 676 677 +1 ============================================ Files 31 31 Lines 1551 1553 +2 ============================================ + Hits 1359 1360 +1 - Misses 192 193 +1
Continue to review full report at Codecov.
|
@mrneatly do you have plans to continue work with PR?
@Smolevich sure, I just need to figure out what's wrong with the code style. As I know from #2438, there were some issues with StyleCI. Is it somehow related to the result of php-cs-fixer in this PR?
I can't push changes to your fork as you haven't given any permissions.
You don't have permissions to push to 'mrneatly/laravel-mongodb' on GitHub. Would you like to create a fork and push to it instead?
Hey @divine I've sent you an invitation to my fork
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.
This seems consistent with what the parent method does in laravel/framework.
Are there any other methods that might need this change?
Could you add a non-regression test to RelationsTest?
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.
Is there a reason for not using InteractsWithPivotTable::parseIds as in the parent class?
Fix
sync()method not handling properly single model's instances passed via$idsargument