Fix EZP-25366: relation fields aren't copied correctly

[bdunogier]

Fixes http://jira.ez.no/browse/EZP-25366

When copying content with relation fields, relations on the copy were not properly registered. This is fixed by applying the same logic used in createContent() and updateContent(): relations inside fields are recorded during copy, and registered using the persistence later at the end.

TODO

• Handle other versions
• Check if something must be done about existing relations
• Tests
• Add integration test for Content Copy
• Add integration test for Subtree Copy

[bdunogier]

Ping @joaoinacio & @andrerom

[crevillo]

short notation? :)

[andrerom]

Looks good, as for LocationService->copySubtree() seen any ways yet on how we can tackle it there?

[bdunogier]

Looks good, as for LocationService->copySubtree() seen any ways yet on how we can tackle it there?

$relationProcessor->updateContentRelations() needs to be called after $contentHandler->copy().

Unfortunately, subtreeCopy is made at persistence level, meaning that we do not have the list of copied content. we could of course get it, but it would be quite heavy, and I would not do that lightly...

[joaoinacio]

May be wrong here as I didn't dig deep into the tests, but it seems this could have the same problem as a recent issue, whereby relations from old versions are taken into account (kept in object_link) when they shouldn't.

Example:

  v1 | eng-GB | -> rel A
  v2 | fre-FR | -> rel B
- v3 | eng-GB | -> rel C

Correct content relations would be B , C but not A.

[joaoinacio]

Also, I'm left wondering if this solution be helpful in the context of https://jira.ez.no/browse/EZP-24540 ?

[bdunogier]

May be wrong here as I didn't dig deep into the tests, but it seems this could have the same problem as a recent issue, whereby relations from old versions are taken into account (kept in object_link) when they shouldn't.

This bugfix doesn't really intend to fix this, to be frank.

[andrerom]

Updated approach, review ping @bdunogier @Plopix @crevillo

[Plopix]

tested with a command like this:

        // subtree
        $repo->getLocationService()->copySubtree($source->location, $dest->location);

        // simple
        $locationStruct = $repo->getLocationService()->newLocationCreateStruct($dest->location->id);
        $repo->getContentService()->copyContent($source->content->contentInfo, $locationStruct);

And also tested in the original context of the explained bug in the issue.

ping @andrerom @bdunogier => Nice job guys ! It works!

[bdunogier]

Great job @andrerom.

+1

[andrerom]

Great job @andrerom.

Thanks, but did actually miss adding 1 test in RelationSearchMultivaluedBaseIntegrationTest, which given it was a duplicate of RelationSearchBaseIntegrationTest made me change them into 1 trait during last rebase. So hopefully even better now :)

Side: Failure is because of behat, issues with some other unrelated merges afaik, should be checked before next sub release.

[bdunogier]

RelationSearchMultivaluedBaseIntegrationTest, which given it was a duplicate of RelationSearchBaseIntegrationTest made me change them into 1 trait during last rebase

Yes, this is one part of tests where we have a big debt to pay.

[yannickroger]

+1

[bdunogier]

Shouldn't you squash the integration tests with the bugfix ? We do backport the tests as well, unless I am mistaken.

[andrerom]

I'll do that on the back port, in case further back ports, thanks for reviewing :)