Fix copy plugins

[yakky]

the low level method CMSPlugin.copy_plugin messes up the reference to the old plugin changing it to the new one
This has no consequence if the old plugin instance is not "reused", but it may affect lower level use (like in blueprint)

[yakky]

@ojii @mkoistinen @evildmp @stefanfoulis @czpython Could you test this for possible regressions?

[ojii]

aren't those two lines redundant?

[yakky]

id and pk are not directly related to each other after loading the model from the storage so we need to update both

[ojii]

Model.pk is a proxy to the primary key field: https://github.com/django/django/blob/89e2c60f4396241c667b7a1de37765b7c96d702f/django/db/models/base.py#L554

Updating id should also update pk (or vice versa)

[yakky]

plugin_instance is a derived class, so pk points to cmsplugin_ptr_id (the derived class primary key) while id is the primary key of the parent class

[ojii]

then why are they both set to the same value?

[yakky]

To keep the object consistent at runtime
Anyway, you should probably ask @ojii as he wrote those two lines in bab4029 :D

[ojii]

I'll shut up about this now. Seeing this just confused me as it doesn't feel right and I hoped that because you touched code near it maybe you'd have some insight as to what's going on.

[yakky]

My understanding is that this is required to return a functional / correct object with right values (plugin_instance is returned and is later used by other functions) without hitting the database again as Django ORM does not make this magic for us.
It's ugly? Yes, I think so.
We should investigate this further for 3.1? I totally agree

[motleytech]

I love it when this happens... :-)

[ojii]

code looks good to me. don't have a real project to properly test this for regressions though.

[jsma]

Was asked to look at this in #3637, I'm not clear on what bug this is solving but have no doubts there is a bug ;) Is there a ticket I'm missing that explains the details of this? As noted in #3637, the issue there is that plugin ordering is thoroughly screwed up on publish and that bug was introduced in the develop branch after the django-treebeard changes were merged. It doesn't appear that this PR addresses that issue.

[yakky]

@jsma yeah, after checking your PR the issue is slightly different, nevermind: I though it could be caused by the leak, but it's not the case

[mkoistinen]

OK, I've tested this on an existing project with some pretty custom and deeply-nested plugin setups. Seems to work! Great job!