New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copying page w/ record with uid smaller than parent fails #1670

Open
andreaswolf opened this Issue Feb 11, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@andreaswolf
Copy link

andreaswolf commented Feb 11, 2019

The problem

Imagine the following situation:

  • a page with a Flux container with uid X
  • a content element with uid Y in the container (colPos = X * 100)
  • Y < X (i.e. the content was created before the container and later moved inside)

Copying that page will fail with a message "The record tt_content:X was designated as a Flux parent for tt_content:NEW… during a copy operation, but the designated parent record does not appear to have been copied. It is possible this is caused by a third-party hook subscriber somehow setting invalid values in DB."

Analysis

This happens due to the nature of the copy process:

  • copying a page in TYPO3 triggers DataHandler with a corresponding cmdmap (command map) that contains a copy operation for this one page
  • this cmdmap triggers a call to DataHandler::copyPages()
    • this copies the page, and then triggers a copy operation for each record on the page, ordered by uid (DataHandler::copyRecord())
    • each of these copy operations creates its own child instance of DataHandler, with a corresponding datamap
      • after each of these copy operations, Flux' hook implementation DataHandlerSubscriber::processDatamap_afterDatabaseOperations() is invoked and tries to remap the colPos value to the new parent
      • this fails because the record with uid Y will be copied after the record with X, but the hook looks for Y's copy directly after X was copied

Solution(s)

One viable solution IMO is to keep a stack of all these "failed" records, and remap them after each operation.

Another would be to do all these remappings only when the cmdmap is finished. For that, the mentioned hook implementation should only record the "records to be remapped" and another hook (to be determined, probably in DataHandler::process_cmdmap()) would do the actual mapping then.

@NamelessCoder

This comment has been minimized.

Copy link
Member

NamelessCoder commented Feb 11, 2019

@andreaswolf can you give this one a second test while you have #1663 applied? Unfortunately I have not had time to test it myself but if you can add a 3rd positive vote I don't have any objections to merging that PR.

@andreaswolf

This comment has been minimized.

Copy link
Author

andreaswolf commented Feb 11, 2019

I just tested our exact failure case and it works perfectly with #1663 applied. I did not do a code review etc., however.

@NamelessCoder

This comment has been minimized.

Copy link
Member

NamelessCoder commented Feb 11, 2019

Thank you Andreas! I'll take care of the code review and CGL adjustments before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment