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

Many to Many behaviour #2871

Open
firstactivemedia opened this Issue Oct 1, 2014 · 27 comments

Comments

Projects
None yet
@firstactivemedia

firstactivemedia commented Oct 1, 2014

The way many to many behaves, does not appear to be very intuitive, and is not nice to use. for example

$tag1 = \Tag::findFirst('id=1');
$tag2 = \Tag::findFirst('id=2');
$tag3 = \Tag::findFirst('id=3');

// create a post
$post = new \Post();
$post->tags = array($tag1, $tag2);
$post->save();

// edit a post
$post = \Post::findFirst('id=1'); // the same post that was created earlier
$post->tags = array($tag1, $tag3);
$post->save();

Will tend to mean the post ends up with all three tags, possibly even with an extra copy of tag1, rather than just 2 as might be expected, and how Doctrine works.

Is this a bug, or intended behaviour?

Some official word on this would be great, seems to attract a few people asking about it http://forum.phalconphp.com/discussion/2190/many-to-many-expected-behaviour

thanks :)

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@prodigga

This comment has been minimized.

Show comment
Hide comment
@prodigga

prodigga commented Oct 10, 2014

Bump...

@prodigga

This comment has been minimized.

Show comment
Hide comment
@prodigga

prodigga commented Oct 12, 2014

Bump 👍

@prodigga

This comment has been minimized.

Show comment
Hide comment
@prodigga

prodigga commented Oct 16, 2014

Bump :)

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Oct 16, 2014

Collaborator

Not a bug, but probably something that can be improved with an array_unique

Collaborator

ghost commented Oct 16, 2014

Not a bug, but probably something that can be improved with an array_unique

@firstactivemedia

This comment has been minimized.

Show comment
Hide comment
@firstactivemedia

firstactivemedia Nov 3, 2014

@phalcon - are you meaning an array_unique in the c code? Because if you mean in our php, I am a little unclear where it would go, as we are assigning an array with unique elements

firstactivemedia commented Nov 3, 2014

@phalcon - are you meaning an array_unique in the c code? Because if you mean in our php, I am a little unclear where it would go, as we are assigning an array with unique elements

@lfbittencourt

This comment has been minimized.

Show comment
Hide comment
@lfbittencourt

lfbittencourt Dec 5, 2014

Talk about other frameworks here may be not too polite, but... Can't Phalcon have a simple sync method like Laravel has?

You may also use the sync method to attach related models. The sync method accepts an array of IDs to place on the pivot table. After this operation is complete, only the IDs in the array will be on the intermediate table for the model:

$user->roles()->sync(array(1, 2, 3));

Refer to http://laravel.com/docs/4.2/eloquent#inserting-related-models.

lfbittencourt commented Dec 5, 2014

Talk about other frameworks here may be not too polite, but... Can't Phalcon have a simple sync method like Laravel has?

You may also use the sync method to attach related models. The sync method accepts an array of IDs to place on the pivot table. After this operation is complete, only the IDs in the array will be on the intermediate table for the model:

$user->roles()->sync(array(1, 2, 3));

Refer to http://laravel.com/docs/4.2/eloquent#inserting-related-models.

@prodigga

This comment has been minimized.

Show comment
Hide comment
@prodigga

prodigga Dec 5, 2014

I mean, sure, but its really not necessary if things worked correctly?

You could simply retreive your user model, and array push "role" entries into that "users" "roles" array. Or you could set the users roles array to an empty array (essentially removing all roles from the user), and array push roles 1 2 and 3 as with your example. Invoke save on the user and you are done..

Refer to my post here:
http://forum.phalconphp.com/discussion/2190/many-to-many-expected-behaviour#C12840

Am I the only one seeing how broken this all is? There seems to be so much confusion around this. None of this works as expected lol.

prodigga commented Dec 5, 2014

I mean, sure, but its really not necessary if things worked correctly?

You could simply retreive your user model, and array push "role" entries into that "users" "roles" array. Or you could set the users roles array to an empty array (essentially removing all roles from the user), and array push roles 1 2 and 3 as with your example. Invoke save on the user and you are done..

Refer to my post here:
http://forum.phalconphp.com/discussion/2190/many-to-many-expected-behaviour#C12840

Am I the only one seeing how broken this all is? There seems to be so much confusion around this. None of this works as expected lol.

@lfbittencourt

This comment has been minimized.

Show comment
Hide comment
@lfbittencourt

lfbittencourt Dec 6, 2014

@prodigga No, you're not. I agree with you :-)

My comment is just a potential next step, because there's no need to load your associated entities from database before attach them to your entity. I mean, in real life you don't SELECT id FROM table WHERE id = 1. All you need are references, like Doctrine and Eloquent do.

lfbittencourt commented Dec 6, 2014

@prodigga No, you're not. I agree with you :-)

My comment is just a potential next step, because there's no need to load your associated entities from database before attach them to your entity. I mean, in real life you don't SELECT id FROM table WHERE id = 1. All you need are references, like Doctrine and Eloquent do.

@prodigga

This comment has been minimized.

Show comment
Hide comment
@prodigga

prodigga Dec 7, 2014

@lfbittencourt very good point! @phalcon just tagging phalcon to see if we can get a response, this issue is buried somewhere in the closed list

prodigga commented Dec 7, 2014

@lfbittencourt very good point! @phalcon just tagging phalcon to see if we can get a response, this issue is buried somewhere in the closed list

@Pajamaman

This comment has been minimized.

Show comment
Hide comment
@Pajamaman

Pajamaman Jan 2, 2015

@phalcon said:

Not a bug, but probably something that can be improved with an array_unique

Seems like a bug to me.

Pajamaman commented Jan 2, 2015

@phalcon said:

Not a bug, but probably something that can be improved with an array_unique

Seems like a bug to me.

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay
Member

sergeyklay commented Jan 2, 2015

@fl0pp

This comment has been minimized.

Show comment
Hide comment
@fl0pp

fl0pp Apr 1, 2016

Have someone come up with a solution to this issue? It will do with plain PHP-code...

fl0pp commented Apr 1, 2016

Have someone come up with a solution to this issue? It will do with plain PHP-code...

@prodigga

This comment has been minimized.

Show comment
Hide comment
@prodigga

prodigga Apr 1, 2016

This small issue steered me away from Phalcon a year ago.

prodigga commented Apr 1, 2016

This small issue steered me away from Phalcon a year ago.

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Apr 1, 2016

Member

Well i will look into it when i have time, but to be honest i created service myself to create/update many to many relation records.

Member

Jurigag commented Apr 1, 2016

Well i will look into it when i have time, but to be honest i created service myself to create/update many to many relation records.

@fl0pp

This comment has been minimized.

Show comment
Hide comment
@fl0pp

fl0pp Apr 2, 2016

@Jurigag could you please share that code with me? I consider doing it myself, but if you have already done it, I would be happy to use it!

fl0pp commented Apr 2, 2016

@Jurigag could you please share that code with me? I consider doing it myself, but if you have already done it, I would be happy to use it!

@fl0pp

This comment has been minimized.

Show comment
Hide comment
@fl0pp

fl0pp Apr 5, 2016

I created the following method that I placed in my base model: http://pastebin.com/MikKbSFj

fl0pp commented Apr 5, 2016

I created the following method that I placed in my base model: http://pastebin.com/MikKbSFj

@makerlabs

This comment has been minimized.

Show comment
Hide comment
@makerlabs

makerlabs Apr 5, 2016

Contributor

Wouldn't it be easier to just remove all the relations between the current post and tags and then to add them again this is how I normally deal with this kind of problem when it comes to many-to-many relations. Even when u look from the UI side its always either show all the related tags already added with an option to add another one. So either u use ajax and add only the one or two new tags or submit all the tags with a normal request again and use the method which I described above.

Contributor

makerlabs commented Apr 5, 2016

Wouldn't it be easier to just remove all the relations between the current post and tags and then to add them again this is how I normally deal with this kind of problem when it comes to many-to-many relations. Even when u look from the UI side its always either show all the related tags already added with an option to add another one. So either u use ajax and add only the one or two new tags or submit all the tags with a normal request again and use the method which I described above.

@firstactivemedia

This comment has been minimized.

Show comment
Hide comment
@firstactivemedia

firstactivemedia Apr 6, 2016

Wouldn't it be easier to just remove all the relations between the current post and tags and then to add them again

The original post was more intended to find out of Phalcon is working as intended. Seems the official line is that it is, so no bug...?

Unsure if I have been spoilt by Doctrine doing more for me than phalcon, or fallen into some kind of anti-pattern / over-reliance on the ORM!

firstactivemedia commented Apr 6, 2016

Wouldn't it be easier to just remove all the relations between the current post and tags and then to add them again

The original post was more intended to find out of Phalcon is working as intended. Seems the official line is that it is, so no bug...?

Unsure if I have been spoilt by Doctrine doing more for me than phalcon, or fallen into some kind of anti-pattern / over-reliance on the ORM!

@angelvega93

This comment has been minimized.

Show comment
Hide comment
@angelvega93

angelvega93 Apr 7, 2016

I created the following method that I placed in my base model: http://pastebin.com/MikKbSFj

It works fine 👍 but it would be even better if you could do like laravel 5

$article->sync('categories', [1, 2 => ['hidden' => 1], 3])

angelvega93 commented Apr 7, 2016

I created the following method that I placed in my base model: http://pastebin.com/MikKbSFj

It works fine 👍 but it would be even better if you could do like laravel 5

$article->sync('categories', [1, 2 => ['hidden' => 1], 3])

@makerlabs

This comment has been minimized.

Show comment
Hide comment
@makerlabs

makerlabs Apr 7, 2016

Contributor

@firstactivemedia sorry I was just referring to the resolution which @phalcon provided I wasn't talking about if its a bug or not :) But when u talk about object oriented approach than yeah I totally agree about the fact that if you assign an array to a object property it should be overriten like it would be in normal object.

Contributor

makerlabs commented Apr 7, 2016

@firstactivemedia sorry I was just referring to the resolution which @phalcon provided I wasn't talking about if its a bug or not :) But when u talk about object oriented approach than yeah I totally agree about the fact that if you assign an array to a object property it should be overriten like it would be in normal object.

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Apr 17, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

stale bot commented Apr 17, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Apr 17, 2018

@StudioMaX

This comment has been minimized.

Show comment
Hide comment
@StudioMaX

StudioMaX Apr 17, 2018

This is still relevant.

StudioMaX commented Apr 17, 2018

This is still relevant.

@stale stale bot removed the stale label Apr 17, 2018

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Jul 16, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

stale bot commented Jul 16, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Jul 16, 2018

@StudioMaX

This comment has been minimized.

Show comment
Hide comment
@StudioMaX

StudioMaX Jul 16, 2018

Still doesn't work.

StudioMaX commented Jul 16, 2018

Still doesn't work.

@stale stale bot removed the stale label Jul 16, 2018

@stale

This comment has been minimized.

Show comment
Hide comment
@stale

stale bot Oct 14, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

stale bot commented Oct 14, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Oct 14, 2018

@prodigga

This comment has been minimized.

Show comment
Hide comment
@prodigga

prodigga Oct 14, 2018

Lol. So, is phalcon dead?

prodigga commented Oct 14, 2018

Lol. So, is phalcon dead?

@stale stale bot removed the stale label Oct 14, 2018

@niden

This comment has been minimized.

Show comment
Hide comment
@niden

niden Oct 15, 2018

Member

Not at all. We just don't have a lot of resources to fix everything. Some of the issues have been opened a long time ago and we have a bot that closes them automatically. For a good number of these issues they are either forgotten, fixed or not pursued. The rest of them we reopen them manually since they are indeed issues we need to address.

Member

niden commented Oct 15, 2018

Not at all. We just don't have a lot of resources to fix everything. Some of the issues have been opened a long time ago and we have a bot that closes them automatically. For a good number of these issues they are either forgotten, fixed or not pursued. The rest of them we reopen them manually since they are indeed issues we need to address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment