magento-hackathon / m2-content-provisioning

Configure CMS (pages or block) entries via XML and define how them should be deployed.
MIT License
45 stars 19 forks source link

Media Files: List of fetched media files are overridden #22

Closed roma-glushko closed 5 years ago

roma-glushko commented 5 years ago

Describe the bug Firegento\ContentProvisioning\Model\Config\Parser\ParserChain collects arrays with CMS content attributes from different parsers and use array_merge() function to merge them all together. array_merge() function replaces the keys of values if they are duplicated and this is fine for attributes like is_active. However, this leads to an issue if we take media_files key. We will get media_files overridden if there is more then one parser that return this key.

This is a case when project uses Firegento_ContentProvisioning with TechDivision_PageDesignerContentProvisioning (https://github.com/techdivision/pagedesigner-content-provisioning)

To Reproduce Steps to reproduce the behavior:

  1. Install Firegento_ContentProvisioning v1.2.1
  2. Install TechDivision_PageDesignerContentProvisioning v.1.1.1
  3. Try to add to the maintanence a CMS block that has images in HTML (like {{media}} directives) and in the PageDesigner encoded JSON (like {{media}} directives or {{widget}} instances with images)

Expected behavior Images from HTML and JSON are collected all togehter and deployed

Actual behavior Only images from the last called parser (a parser from TechDivision_PageDesignerContentProvisioning) are present and deployed

vadimjustus commented 5 years ago

Hi @roma-glushko, I've tried to reproduce this issue with some integration tests (this are more unit tests - I know). Can not reproduce the issue with merging media files. Do I misunderstood it? See here: https://github.com/magento-hackathon/m2-content-provisioning/pull/23/files

-- Update --

After re-reading your description I understood :) Yes, this could be an issue - but in case of using PageDesigner the content value is completely obsolete, since after saving the page/block a Plugin of PageDesigner generates the whole content out of the JSON data. So only PageDesigner JSON and its media files are relevant.

roma-glushko commented 5 years ago

Hi @vadimjustus This makes sense. Thank you for this explanation. Than I will close this task and solution should be applied in a scope of TechDivision_PageDesignerContentProvisioning