-
Notifications
You must be signed in to change notification settings - Fork 39
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
Product ID inserted into page does not preserve original Magento1 logic for replacing illegal characters #13
Comments
…tween the Magento1 and Magento2 module. This ensures that data will continue to get pulled into Magento2 stores when the product ID has a perfectly legal period in it.
I suspect you'll be solving one problem and discovering another here... we did a migration to Magento 2 earlier this year, and there actually appears to be a change in the parser on the BV servers that ingest the feeds that doesn't recognize the period character in category external ids... there is no logical reason for this, but we were told by the internal BV engineers, point blank, that periods are not considered valid and that including the character in external ids will always throw an error. This has proven to be mostly true... it only seems to throw a parsing error when creating NEW categories, but it accepts existing categories with periods in the external id just fine. Additionally, it's not like we can easily revert to using numeric ids (for one thing, they don't always remain the same between separate installs without a db migration... and that's pretty icky in Magento 2, especially in cloud environments), as the magento category structure ends up flattened by the BV parser to the root category when that is used. In your particular case, I strongly suspect that the existing category external IDs with periods in them will be parsed just fine, BUT if you create a new category using the module without this period-to-underscore swap, the entire feed will be invalidated when it tries to create a new external id for a category on the BV side. Fun. |
We are currently in the process of migrating to Magento2. We use BazaarVoice (BV)'s Connector module in Magento1. Once the Magento2 version is installed, though, it does not pull in any of our customer reviews, ratings, and questions.
The reason this is happening is because the Magento2 module does not preserve the same logic for replacing illegal characters in product IDs from Magento1. I stepped through the code and compared both of the modules.
Magento1:
Magento2:
The differences:
There are two important differences to note, and they are the reason why no BV information displays on our under-development Magento2 store.
The first screenshot in each group of screenshots displays a section where there is a place cordoned off, via comments, where the original author suggests that the section is a good place for customizations. In Magento1, there is a blank newline between both; there is no special customization. In Magento2, there is. In Magento2, it adds nearly identical logic from the
replaceIllegalCharacters
method.The
replaceIllegalCharacters
method in Magento1 uses/[^\w\d\*-\._]/s
as a regular expression. The method by that name in Magento2 modifies it by removing the period:/[^\w\d\*-\-_]/s
. Magento2 removes the period as a valid character that should not be replaced, which means unlike in Magento1, Magento2 incorrectly replaces the period.The problem:
These two differences are the reason no data gets pulled into our Magento2 store.
This should be fixed to remain true to the logic and intent from Magento1. Without the fix, this means configurable product data that pulls in data for Magento1, under the allowed characters that BV uses as a format, for example, will not work when the same exact ID is used for Magento2. That is a lot of data to lose. An example between the differences in the product IDs between Magento1 and Magento2 look like this:
conf.CM12345
conf_bv46_CM12345
I have a pull request incoming that fixes this issue. I will also reply to the email from BV support, which was taking too long to move forward on this issue.
The text was updated successfully, but these errors were encountered: