magento-engcom / import-export-improvements

Open Software License 3.0
31 stars 29 forks source link

ImportExport: BEHAVIOR_REPLACE deletes products? #89

Open piotrekkaminski opened 6 years ago

piotrekkaminski commented 6 years ago

From @paales on August 4, 2016 14:51

Preconditions

  1. Magento 2.1.0

    Steps to reproduce

  1. Import Product with BEHAVIOR_REPLACE set
  2. Import Product with BEHAVIOR_REPLACE set again

    Expected result

  1. Same product ID is used, but all the values provided by the import are replaced

    Actual result

  1. Product gets deleted and recreated...

It REALLY shouldn't delete the product because all other information related to this product gets lost. If a merchant has written anything store view specific, we want to retrieve reports by product ID, reviews placed on this product, etc.

I'm not sure how something like this happened, the BEHAVIOR_REPLACE doesn't mean replace the product, it means, replace complex values like custom options, configurable products or category associations (wtf?).

I will update this issue with suggestions how to solve this mess and how to solve the importer issues.

Copied from original issue: magento/magento2#5993

piotrekkaminski commented 6 years ago

From @paales on August 4, 2016 15:12

Replace https://github.com/magento/magento2/blob/develop/app/code/Magento/CatalogImportExport/Model/Import/Product.php#L920

With:

$this->_saveProductsData();

The complete _replaceProducts method should be deleted from the class, git blame should be started and the developer responsible should treat the complete company apple pie! 😋

piotrekkaminski commented 6 years ago

From @paales on August 5, 2016 9:10

It seems to be not as easy as removing the above lines. I'm getting the following when importing configurable products. Will report back when I know more.

SQLSTATE[23000]: Integrity constraint violation: 1451 Cannot delete or update a parent row: a foreign key constraint fails (`promopost`.`catalog_product_super_attribute_label`, CONSTRAINT `FK_309442281DF7784210ED82B2CC51E5D5` FOREIGN KEY (`product_super_attribute_id`) REFERENCES `catalog_product_super_attribute` (`), query was: INSERT INTO `catalog_product_super_attribute` (`product_super_attribute_id`,`position`,`product_id`,`attribute_id`) VALUES (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?), (?, ?, ?, ?) ON DUPLICATE KEY UPDATE `product_super_attribute_id` = VALUES(`product_super_attribute_id`), `position` = VALUES(`position`), `product_id` = VALUES(`product_id`), `attribute_id` = VALUES(`attribute_id`)
piotrekkaminski commented 6 years ago

From @paales on August 5, 2016 11:16

Two things:

  1. I didn't solve the integrity constraint violation.
  2. The table it gives the integrity constraint violation on doesn't have to be filled with the importer..

This methods generates the data: https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/app/code/Magento/ConfigurableImportExport/Model/Import/Product/Type/Configurable.php#L698-L724

You can see here that it generates a label for the attribute, but it currently isn't possible to set store view specific labels and we don't even want a label unique to each product.

So for now I've overwritten the method and just emptied it.

piotrekkaminski commented 6 years ago

From @slopukhov on August 23, 2016 8:37

@paales thank you for your feedback.

This is the expected behavior for Replace action. Please read Magento user guide for more details.

I'm closing the ticket. Please feel free to reopen if you have additional questions.

piotrekkaminski commented 6 years ago

it's definitely not intuitive behavior that needs further investigation. @ilol can you look into that?

piotrekkaminski commented 6 years ago

From @ioliinyk on March 29, 2017 10:51

This is bug.

piotrekkaminski commented 6 years ago

From @KrystynaKabannyk on April 18, 2017 15:54

Internal ticket is created MAGETWO-67547.

piotrekkaminski commented 6 years ago

From @magento-engcom-team on September 29, 2017 6:30

@paales, thank you for your report. We've created internal ticket(s) MAGETWO-67547 to track progress on the issue.

piotrekkaminski commented 6 years ago

From @PieterCappelle on December 22, 2017 19:39

I agree with @paales. REPLACE should be refactored or removed. Add/Update is also so wrong in many cases. When running add import it will add images to products, it will not overwrite them. Okay, I can understand but REPLACE is also no option like @paales is saying. In my opinion we should have one command for add/update/replace. :-)

dmanners commented 5 years ago

My opinion here would be that we could consider updating the documentation and tool tips to help describe the settings for the import better.

dmanners commented 5 years ago

So for this task what we are looking for here is 2 things:

  1. Updating the documentation,
  2. Adding UI labels via some form of hint icon,

Both of these should clearly describe what happens with the different flags for the import process. Especially the BEHAVIOR_REPLACE. We will not change the behavior at the moment but instead make the current behavior much more clearer in this ticket.

lorikrell commented 5 years ago

Any updates need in documentation, I'm available to help in Merch Docs and DevDocs.