Need to add node_clone integration to prevent duplicating the uuid.

CommentFileSizeAuthor
#20 1868302-6.x-20.patch1004 bytespwolanin
#18 1868302-18.patch556 bytespwolanin
#13 node_clone-uuid-support-1868302-13.patch1.27 KBsebas5384
#13 node_clone-uuid_support-1868302-13.patch1.27 KBsebas5384
#12 dont-clone-uuids-1868302-12.patch1.02 KBmixxmac
#9 uuid-node-clone-compat-1868302.patch505 bytesethanethan
#8 node-clone-uuid-revisions-3.jpg523.59 KBmixxmac
#3 node-clone-uuid-revisions-2.png110.79 KBmixxmac
#1 node_clone_alter-1868302-1.patch463 byteschrisivens
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chrisivens’s picture

chrisivens CreditAttribution: chrisivens commented
FileSize
node_clone_alter-1868302-1.patch463 bytes

THis patch may or may not add the hook_clone_node_alter() function in the right place but it should prevent the uuid being duplicated with node_clone.

chrisivens’s picture

chrisivens CreditAttribution: chrisivens commented
Status: Active » Needs review
mixxmac’s picture

mixxmac CreditAttribution: mixxmac commented
Status: Needs review » Needs work
FileSize
node-clone-uuid-revisions-2.png110.79 KB

Thanks for adding this patch. I just tested it, and it looks like it does the trick for the uuid field.

It looks like the revision ids and comments are being cloned too though. So, I tried editing your patch to see if the same would work for those fields...

function uuid_clone_node_alter(&$node, $original_node, $method) {
  $node->uuid = '';
  $node->vid = '';
  $node->log = '';
}

I think $node->log = ''; works to prevent the revision comments from being cloned. But, $node->vid = ''; didn't seem to do anything to prevent vuuids from being cloned. So, I think there is still a problem with the revision VUUID being cloned.

The attached image shows the results I was getting when I tested this.

Thanks for adding the patch. If we can fix the vuuid part, then I think that would solve this issue.

chrisivens’s picture

chrisivens CreditAttribution: chrisivens commented

I don't know for certain but something could maybe be done with these bits:

$info = entity_get_info($entity_type);

$vuuid_key = $info['entity keys']['revision uuid'];

$entity->{$vuuid_key} = uuid_generate(); // OR set to null

By setting the vuuid to null it should regenerate in uuid_entity_presave()

mixxmac’s picture

mixxmac CreditAttribution: mixxmac commented

Thanks Chris. It looks like I made a mistake and used "vid" instead of "vuuid". Maybe that's all it needs. I'll try testing this again soon, and hopefully that will fix it.

chrisivens’s picture

chrisivens CreditAttribution: chrisivens commented

If you do get it to work, please let me know. I'd love to get this fixed ready for a deploy I need to do imminently.

mixxmac’s picture

mixxmac CreditAttribution: mixxmac commented

Ok... will do. I'll try testing this again later today, and let you know if it works.

mixxmac’s picture

mixxmac CreditAttribution: mixxmac commented
Status: Needs work » Needs review
FileSize
node-clone-uuid-revisions-3.jpg523.59 KB

I tested the patch with the extra two lines of code that I added. It worked, so this seems to be the fix that is needed in uuid.core.inc.

I don't have a patch for this, but this is the code to fix the problem:

function uuid_clone_node_alter(&$node, $original_node, $method) {
  $node->uuid = '';
  $node->vuuid = '';
  $node->log = '';
}
ethanethan’s picture

ethanethan CreditAttribution: ethanethan commented
FileSize
uuid-node-clone-compat-1868302.patch505 bytes

The change in #8 solves the issue for us, including in translation and revision (ERS) test cases.

I made a patch from #8 and will try to trigger the automated tests again...

ethanethan’s picture

ethanethan CreditAttribution: ethanethan commented

Are these tests being postponed because theres something wrong with the patch? Maybe we need a dependency on node_clone somewhere.

skwashd’s picture

skwashd CreditAttribution: skwashd commented
Project: Universally Unique IDentifier » Node clone
Status: Needs review » Needs work

node_clone isn't not part of core, it is a contrib module. This needs to be fixed in the node_clone module. It doesn't introduce a hard dependency on UUID, so there is nothing stopping the fix landing there.

mixxmac’s picture

mixxmac CreditAttribution: mixxmac commented
Status: Needs work » Needs review
FileSize
dont-clone-uuids-1868302-12.patch1.02 KB

I made a patch for setting uuid, vuuid, and log to NULL in clone_node_prepopulate() and clone_node_save(). I'm not sure if this is necessary anymore for the vuuid and log (node_revisions), because I didn't notice a problem with them when I tested it this time.

Seems to work for me, but please test to confirm. Thanks.

sebas5384’s picture

sebas5384 CreditAttribution: sebas5384 commented
FileSize
node_clone-uuid_support-1868302-13.patch1.27 KB
node_clone-uuidsupport-1868302-13.patch1.27 KB

I reviewed the patch and worked like a charm!

This problem was really messing things up when using UUID, and with others modules like Services UUID.

I just add a simple module_exists() to check if the UUID is enabled to the patch of @mixxmac in #12.

Cheers!

gcassie’s picture

gcassie CreditAttribution: gcassie commented
Status: Needs review » Reviewed & tested by the community

Works well for me.

Maxime Gilbert’s picture

Maxime Gilbert CreditAttribution: Maxime Gilbert commented

Works well with Node export and Features.
Must be added to the next release candidate.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

Why is the module_exists() needed?

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Reviewed & tested by the community » Needs work

needs re-roll for refactoring in [#1316284 ]

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs work » Needs review
FileSize
1868302-18.patch556 bytes

this simple?

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)

committed #18

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Patch (to be ported) » Needs review
FileSize
1868302-6.x-20.patch1004 bytes

  • Commit 70c1d11 on 7.x-1.x, 8.x-1.x by pwolanin: 
    Issue #1868302 by mixxmac, sebas5384, pwolanin: fix for Node clone...
ccb621’s picture

ccb621 CreditAttribution: ccb621 commented
Issue summary: View changes

What stopped/is stopping this from being merged?

Never mind. I see this was merged to the 7.x and 8.x branches.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin as a volunteer and at SciShield commented
Status: Needs review » Fixed

oops, yeah - just failed to mark it fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.