Fix EZP-29116 : Conversions from ezxmltext to ezrichtext fails if ezxmtext have duplicate xhtml ids #36
Conversation
vidarl
force-pushed
the
dryrun-and-contentobject
branch
from
c04068b
to
8424e08
Compare
vidarl
changed the title
|
The tests will fail until ezsystems/ezpublish-kernel#2312 is merged. |
|
@adamwojs : Could you have a look on this one too? I was not able to add you as reviewer |
| * @copyright Copyright (C) eZ Systems AS. All rights reserved. | ||
| * @license For full copyright and license information view LICENSE file distributed with this source code. | ||
| * | ||
| * @version //autogentag// |
There was a problem hiding this comment.
Nitpick: @version is not needed anymore.
| ) | ||
| ); | ||
| } | ||
| function removeComments(DOMDocument $document) |
There was a problem hiding this comment.
Nitpick: Missing new line between methods. Can we also specify the visibility of the method?
There was a problem hiding this comment.
Yeah.. both nitpicks are a result of cut&paste... I'll fix
| @@ -174,8 +141,9 @@ function convertFieldDefinitions($dryRun, OutputInterface $output) | |||
| $output->writeln("Converted $count ezxmltext field definitions to ezrichtext"); | |||
| } | |||
|
|
|||
| function convertFields($dryRun, $contentObjectId, OutputInterface $output) | |||
| function convertFields($dryRun, $contentObjectId, $checkDuplicateIds, OutputInterface $output) | |||
There was a problem hiding this comment.
Nitpick: missing visibility specification for method.
| array( | ||
| new ToRichTextPreNormalize(new Expanding(), new EmbedLinking()), | ||
| new Xslt( | ||
| './vendor/ezsystems/ezpublish-kernel/eZ/Publish/Core/FieldType/RichText/Resources/stylesheets/ezxml/docbook/docbook.xsl', |
There was a problem hiding this comment.
Specifying path relative to __DIR__ is a bit safer for me, especially when it was moved from the command
There was a problem hiding this comment.
I don't think that will work.
If only running tests, you may not have a meta repo. so you will have these paths:
lib/FieldType/XmlText/Converter/RichText.php
vendor/ezsystems/ezpublish-kernel/eZ/Publish/Core/FieldType/RichText/Resources/stylesheets/ezxml/docbook/docbook.xsl
Which means __DIR__ . '/../../../../vendor/ezsystems/ezpublish-kernel/eZ/Publish/Core/FieldType/RichText/Resources/stylesheets/ezxml/docbook/docbook.xsl',
But when you have a meta repo and running the actuall bin/console ezxmltext:convert-to-richtext command, ezpublish-kernel will be in different location relative to ezplatform-xmltext-fieldtype :
vendor/ezsystems/ezplatform-xmltext-fieldtype/lib/FieldType/XmlText/Converter/RichText.php
vendor/ezsystems/ezpublish-kernel/eZ/Publish/Core/FieldType/RichText/Resources/stylesheets/ezxml/docbook/docbook.xsl
Which will require __DIR__ . '/../../../../../../../vendor/ezsystems/ezpublish-kernel/eZ/Publish/Core/FieldType/RichText/Resources/stylesheets/ezxml/docbook/docbook.xsl',
There was a problem hiding this comment.
So, I could add some logic and check whatever vendor/ezsystems/ezpublish-kernel or ../ezpublish-kernel exists, and set the path accordingly, relative to __DIR__
There was a problem hiding this comment.
So, I could add some logic and check whatever vendor/ezsystems/ezpublish-kernel or ../ezpublish-kernel exists, and set the path accordingly, relative to DIR
@vidarl No, in this circumstances let's leave it as is 😉
This fix is dependent on ezsystems/ezpublish-kernel#2312 which actually deals with the xml conversion..
The change in this PR