reordering blocks of copied pages buggy due to BlockRelations

[Remo]

We've found an issue I'm not sure how to fix. Here's how you can reproduce it:

1. Create a new page page1
2. Add a content block block1
3. Add a content block block2
4. Publish changes
5. Copy page1 to page2
6. Edit page1 again
7. Reorder blocks so that block2 comes before block1

This action will cause a change in both pages.
That happens because concrete5 doesn't duplicate the block when we copy a page but rather links them by using the table BlockRelations. This means that we have to duplicate the actual block content when reordering a block. I couldn't find a method which would easily make it possible to really clone a block without creating a relation. Any ideas?

[aembler]

Definitely can't reproduce this. The CollectionVersionBlocks table holds block ordering, and that table is duplicated across pages when a new page is made. No, the actual block content isn't duplicated but it doesn't need to be. Block content is duplicated when necessary when particular edits are made.

[aembler]

This is where that happens: https://github.com/concrete5/concrete5/blob/develop/web/concrete/src/Page/Collection/Collection.php#L1111

[Remo]

Check this video www.ortic.com/2015-10-27_16-28-02.mp4
I manually re-send the rearrange question after I modified the display order of both pages, after sending it, both pages are changed

[Remo]

this query is missing the cID https://github.com/concrete5/concrete5/blob/develop/web/concrete/src/Page/Page.php#L373-L384

[aembler]

Ah, you are right, that code is definitely going to cause problems. Looks like it was written assuming that the block IDs are unique in the CollectionVersionBlocks table, when that is definitely not the case. Nice catch. Not sure why my tests didn't show this but I imagine the way this bug works is that it's intermittent.

I will wait til you have a chance to test whether this fixes the issue for you, but it looks like a good fix.

[Remo]

@aembler fix works on a second site too. The code is a bit ugly to read with those cases joined together, but it's efficient and I wouldn't want to make it easier to read at the cost of running a query per block.

[aembler]

Applied the fix. Thanks!