PerplexDigital / Perplex.ContentBlocks

Block based content editor for Umbraco
MIT License
31 stars 15 forks source link

Content block keys are not changed when page copied #45

Closed glombek closed 3 years ago

glombek commented 3 years ago

When copying a page, all the content blocks content[0].key values are not updated to be unique. This means that when the PropertyValueConverter runs the _nestedContentSingleValueConverter.ConvertIntermediateToObject function, it will use the item from the content cache based on the key.

This issue occurs if you try to retrieve multiple nodes in one request when some of the nodes are a copy of another.

A temporary workaround is to override the referenceCacheLevel to be PropertyCacheLevel.None in the ContentBlocksValueConverter.ConvertIntermediateToObject method.

PerplexDaniel commented 3 years ago

Hmm yeah I believe we also ran into this, when the entire page is copied. When you copy 1 or more blocks in the backoffice we do run some code in Angular to update all content block ids + nested content keys (nested). So I suppose we can look into doing some sort of transformation on ContentService.Copied and update any ContentBlocks properties there. Would be good to check how NestedContent is handling this.

Not sure if we have much time for this soon, but I can imagine this is an annoying one for clients. Workaround is to copy a page, clear all blocks and use the "Copy all blocks" feature instead from the original page.

glombek commented 3 years ago

I can confirm the workaround works to fix existing broken pages too. Copy all blocks, delete all blocks, paste all blocks.

I'll be implementing an event on ContentService.Copied for this project, which I will share here/create as a PR if you're open to it and I find the time!

PerplexDaniel commented 3 years ago

Oh that's right, it would even fix the current ones. Would be great if you can submit a PR to do this on the server when people copy the whole page. FYI the JavaScript code is here which recursively updates all NestedContent keys (of the block itself but also any other properties).

glombek commented 3 years ago

Thanks @PerplexDaniel!

I've fixed this for my current project, but haven't yet had the chance to properly create this as a PR and test outside my own solution.

I'll try and do this one evening, but in case someone else picks this up before me, Here is the code that I'm using https://gist.github.com/glombek/4f57acffae05e5181320fdd0fc61e0dd

PerplexDaniel commented 3 years ago

Great! I'll have a look at this hopefully next week.

PerplexDaniel commented 3 years ago

Unfortunately I have not been able to look at this yet. I'll be off for 2 weeks starting next week so I'll have to delay this a bit longer but it's on the list!

PerplexDaniel commented 3 years ago

Hi @glombek,

I finally got around to testing your code a bit. I made some adjustments so it also works in multilingual scenarios and also updates Nested Content keys when they are nested inside another Nested Content. The JSON value is a bit weird in that case -- a string like "[{ \"key\": \"...\"}]" rather than a real array like [{"key": "..."}] so I made some small adjustments to the parsing code there.

This is released as part of 1.6.3. Thanks for your code and the report!

glombek commented 3 years ago

Thanks @PerplexDaniel! Sorry I never got the chance to PR this myself