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

[WIP] New method linkSync to update junction tables #7103

Closed
wants to merge 4 commits into from
Closed

[WIP] New method linkSync to update junction tables #7103

wants to merge 4 commits into from

Conversation

alex-code
Copy link
Contributor

Allow junction tables to be updated via the relation.

$model-id other-id
1 1
1 2
1 3
1 4

$model->linkSync('relation', [1, 2, 5, 6]);
This would remove the relation to 3 & 4 and insert new relations to 5 & 6.
Based this off laravel->sync() from an article I read.

This is more of a demo to get some thoughts as it would need updating to support relations more strongly.

Allow junction tables to be updated via the relation.
`$model->linkSync('relation', [1, 2, 3, 4]);`
@githubjeka
Copy link
Contributor

A good method. I often use it.
Need tests!

@alex-code
Copy link
Contributor Author

Need to write some tests but should support composite keys now.

The method can take either an array of foreign key ids or an array of foreign models.

e.g.
Using Html::checkboxList you could do,
$model->linkSync('relation', \Yii::$app->request->post((new OtherModel)->formName(), []));
or
$model->linkSync('relation', OtherModel::findAll(\Yii::$app->request->post((new OtherModel)->formName(), [])))

Thoughts?

@Faryshta
Copy link
Contributor

Very good, what is needed for integration?

@samdark
Copy link
Member

samdark commented Jul 22, 2015

Tests, docs.

@Faryshta
Copy link
Contributor

@samdark the name of the method is ok? I think setRelation or syncRelation are easier to understand.

@alex-code can I change the function signature to add $columns = [] and $delete = false ?

@samdark
Copy link
Member

samdark commented Jul 22, 2015

No, method name is not OK. Totally confusing.

@Faryshta
Copy link
Contributor

@samdark which method name would you prefer?

@samdark
Copy link
Member

samdark commented Jul 23, 2015

Something like updateLink. "link" is OK because we have link and unlink methods.

@alex-code
Copy link
Contributor Author

@Faryshta Please do make changes, I put this up so it can be improved and expanded.

@Faryshta
Copy link
Contributor

@samdark I was thinking more along the lines of setRelation since we already have a getRelation method.

@samdark
Copy link
Member

samdark commented Jul 23, 2015

It's not really about relations. It's about junction table...

@alex-code
Copy link
Contributor Author

Maybe linkMultiple, I don't think setRelation says enough about what the function would do.

@cebe cebe added the pr:request for unit tests Unit tests are needed. label Nov 27, 2016
@yii-bot
Copy link

yii-bot commented Nov 27, 2016

Thank you for putting effort in the improvement of the Yii framework.
We have reviewed your pull request.

In order for the framework and your solution to remain stable in the future, we have a unit test requirement in place. Therefore we can only accept your pull request if it is covered by unit tests.

Could you add these please?

Thanks!

P.S. If you have any questions about the creation of unit tests? Don't hesitate to ask for support. More information about unit tests

This is an automated comment, triggered by adding the label pr:request for unit tests.

@samdark samdark modified the milestones: 2.0.x, 2.1.0 Dec 18, 2017
@samdark
Copy link
Member

samdark commented Apr 23, 2019

Won't make it into 2.0.

@samdark samdark closed this Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants