Mass delete deletes all products

patrick-bigbridge commented 6 years ago

On the product page, if you select "Delete" twice, all products will be removed.

Preconditions

Magento 2.1.11 & 2.3.x EE & 2.3.x CE(2.3-develop too)
Have some products (e.g. 10)
Created products with name&sku test 1, test 1-2, test 1-2-3
Steps to reproduce
Go to backend, Products / Catalog
Make a selection of some product (using a Search by keyword with 'test 1' value) and select checkbox(for 'select all' option)
Select action "Delete"
Click "OK" in the confirmation popup
At this point do not wait until Magento is done, but while it is still running, do:
Select action "Delete" again
Click "OK" in the confirmation popup

Magento's request will run for a long time(in case with a lot of products). If you check the database (select(count(*) from catalog_product_entity), you will see that the number of products keeps decreasing.

Expected result

I expect that Magento either disables user input while deleting products, or that is keeps its selection until after the products are deleted.

Actual result

All products deleted. GIF(testing scenario): Peek 2019-10-08 15-07

allanpaiste commented 6 years ago

Was able to repeat on 2.1.13 & 2.2.4; it does require you to be either very fast (use ENTER on the confirmation window) or to have slow communication with the server (could be conditioned with different OS-level network tools)

michelangeloc commented 6 years ago

I was able to reproduce it on Magento Commerce 2.2.2

RossCC commented 6 years ago

This appears to be our issue on 2.2.1, as described here. https://community.magento.com/t5/Magento-2-x-Technical-Issues/Magento-systematically-deleting-all-products-one-by-one-on-its/m-p/97335#M5366

AidanThreadgold commented 6 years ago

I have reproduced this on Magento Commerce 2.2.3.

ericvhileman commented 6 years ago

Confirmed in versions | 2.1.11 | 2.2.1 | 2.2.2 | 2.1.9

philippsander commented 6 years ago

Probalby not worth another issue, but I had the same problem in the customer grid

denteblu commented 6 years ago

In PR #15985 we have fixed the same issue in Customer, Page, CMS Block, and Search Synonyms Grids.

novikor commented 6 years ago

Already fixed by Core team in 2.2-develop (future 2.2.6): a892d87bc3b7fe2819ccdff01ef04b30a72fbe70, 70cbd3cf8b46d18824b316c55c93ea68ffbc47d3.

I will create a backport for Magento 2.1

engcom-backlog-nickolas commented 6 years ago

Hello @patrick-bigbridge, thank you for your report. We've acknowledged the issue and added to our backlog.

sidolov commented 6 years ago

Hi @patrick-bigbridge. Thank you for your report. The issue has been fixed in magento/magento2#16702 by @novikor in 2.1-develop branch Related commit(s):

The fix will be available with the upcoming 2.1.15 release.

magento-engcom-team commented 6 years ago

@patrick-bigbridge Apparently it's now fixed in 2.1-develop and 2.2-develop. We are closing the ticket now.

@novikor Thanks a lot. Fix for 2.3 may be useful as well ☺️

juharintanen commented 6 years ago

Is it possible that this is still active for 2.2.5 ? This has happened total of 3 times already in 2 different stores and I can't figure out the reason why..

philippsander commented 6 years ago

@juharintanen look at the tags and versions mentioned in the comment-scetion I sume it's not fixed in 2.2.5

2.2.5 was published on July 1, 2018.

This fix will probably go live with 2.2.6

maksim-grib commented 5 years ago

This issue is more general, it affects not only delete mass action but all mass actions that internally make use of Magento\Ui\Component\MassAction\Filter class. Even for mass action from 3rd party vendors, as long as they are using the same approach. This a form of race condition issue.

Because of this bug all products were disabled on one of our projects. Here is the other way how you can reproduce it:

Imagine two people are working on Magento catalog simultaneously and they opened product grid page roughly at the same time.

1) First guy tries to remove product with id 1 via mass action (or from product edit page, this is irrelevant in this case). 2) Second guy tries to disable the same product via mass action (in this case doing it via mass action is important part). 3) Since request of the first guy will be processed first, product will be already removed at the time when request of the second guy will be processing. Because of this all ids will be returned by UI filter component and mass action will be applied to all products.

To be honest it is weird to see that such a critical issue was fixed only recently.

Milesy1 commented 5 years ago

This is NOT a bug.

Magento have a VERY big problem with their testing methodolgy.

The future maxima is greater than the past maxima.

You do NOT test/calibrate new releases of established software against a PAST benchmark, you test against a HIGHER benchmark thereby revealving hidden problems.

This has caused me SERIOUS issues.

samuelmafra commented 5 years ago

This BUG is on 2.2.5.

Milesy1 commented 5 years ago

Learn how to test PROERLY

nbennett25 commented 5 years ago

Also able to reproduce this on 2.2.5 - and confirmed in a clean install with sample data.

hostep commented 5 years ago

@nbennett25 an all others, it looks like the fix made it in 2.2.6, like was previously already mentioned, no idea where you got 2.2.5 from to be honest ...

nbennett25 commented 5 years ago

@nbennett25 an all others, it looks like the fix made it in 2.2.6, like was previously already mentioned, no idea where you got 2.2.5 from to be honest ...

Thx @hostep - I'll download and test with 2.2.6. You can download v2.2.5 (and others) from Magento under 'Release archive.'

niners52 commented 5 years ago

Just happened in 2.3.1

patrick-bigbridge commented 5 years ago

@niners52 That is terrible news! There was an actual fix!

I just tried to replicate the original problem on a 2.3.1 and I could not replicate it. Can you tell me exactly what you do to set it off?

niners52 commented 5 years ago

@patrick-bigbridge The user did exactly what you mentioned in the original post. They got impatient and hit delete again. I ended up restoring from a previous backup and importing all of the orders that came over since. It was a nightmare.

patrick-bigbridge commented 5 years ago

@niners52 Yes this is terrible having to restore all those orders.

But, did you say orders? My original post is not about orders. It is about products. Now it is true that the bug mentioned here applied to other grids as well (it is a generic grid problem). But (mass) deleting orders is not a standard Magento feature.

Do you have an extension installed that allows the user to do this, or do you mean something else?

niners52 commented 5 years ago

@patrick-bigbridge Sorry I worded that poorly. It was the products that were deleted. What I ended up doing was restoring to a previous version of the site since that had all of the products.

The problem with that restore was when I did that I lost all of my most recent orders over the last 5 days since the restore was from before that so I had to export and reimport those.

patrick-bigbridge commented 5 years ago

Thanks. Terrible that this bug still exists. Could not reproduce it myself though.

kapil-infobeans commented 5 years ago

It should be reopened, It still exist in Magento 2.2.5. Instead of importing product, there is good way to do it. You just have to do below steps.

Put snapshot of old correct database, that you can get from hosting providers if scheduled backup's are there.
Put corrupted database on same mysql server.
Find casecade in mysql for catalog_product_entity table, So you will get to know which tables data you need to copy to corrupted database on previous id(s).
prepare sql queries to select data from snapshot db for inserting into corrupted database on same entity_id(s), prepare a .sql file of queries.
Import .sql file to corrupted database.

That's all, we have restored 7000+ products in 30 minutes without having overhead of re-import and all.

patrick-bigbridge commented 5 years ago

@kapil-infobeans It was fixed in 2.2.6. In what version are you still seeing the problem?

kapil-infobeans commented 5 years ago

Thanks for quick follow up @patrick-bigbridge

We are using 2.2.5 version of Magento EE, I am not sure about more latest version. Could you please share link of fixes? If that's the case here I will change my statement instantly.

patrick-bigbridge commented 5 years ago

Ok, the link can be found earlier in this issue: https://github.com/magento/magento2/issues/15935#issuecomment-403922901

kapil-infobeans commented 5 years ago

Thanks @patrick-bigbridge, I will try with this fix.

youphp commented 5 years ago

@kapil-infobeans I encountered this bug in magento 2.2.6. This bug almost deleted all product data in my store. I am the database backup on June 14. I tried to restore to this database and export all product data and then import, but since I migrated the product data from magento1.9, I have encountered a lot of import errors. So I gave up this method to restore my product. I want to restore my product by exporting the product data sheet for the June 14 database and then overwriting my current product data sheet. I am very anxious now, can you tell me how you recovered the detailed process?

kapil-infobeans commented 5 years ago

@youphp As I have explained above. You just need to restore deleted products same entity_id. You just have to do below steps.

Put snapshot of old correct database, that you can get from hosting providers if scheduled backup's are there.

Put corrupted database on same mysql server.

Find casecade in mysql for catalog_product_entity table, So you will get to know which tables data you need to copy to corrupted database on previous id(s).

prepare sql queries to select data from snapshot db for inserting into corrupted database on same entity_id(s), prepare a .sql file of queries.

Import .sql file to corrupted database.

That's all, we have restored 7000+ products in 30 minutes without having overhead of re-import and all.

SolsWebdesign commented 5 years ago

Hello, this happened with a client on Magento 2.2.6 and I had a hard time reproducing the problem. I wondered, did the client accidentally choose the "Select All" instead of the "Select All on this page" from the select drop-down in backend or did this really occur?

I decided to "help" the testing a bit by emptying the getFilterIds() array but that would just work fine. I then wrote a kind of stress test and added the fix provided in the pull/15985 with and without the avoidEmptyFilter (that seemed to be true every time anyway) and it would go right 99% of the time.

Until it went wrong and I found it did go into the if (!empty($idsArray) || $this->avoidEmptyFilter) { .. } and the if (!empty($idsArray)) { .. } supplied by the suggested pull/15985 at all. So it looked like checking if $idsArray = $this->getFilterIds(); was filled (or not) did not make a difference which made me think.

I went back to the getFilterIds() function and looked at the line if ($this->getDataProvider() instanceof \Magento\Ui\DataProvider\AbstractDataProvider) { // Use collection's getAllIds for optimization purposes. $idsArray = $this->getDataProvider()->getAllIds(); } else { ... It almost looks like that sometimes it ends up in this $idsArray = $this->getDataProvider()->getAllIds(); line...

To test this I took the original file vendor/magento/module-ui/Component/Massaction/Filter.php and commented out the whole collection->addFieldToFilter(...) line in the getCollection function (so it does not get filtered at all), selected 2 products in the backend and did a (MassAction) Delete and watched as the whole database was slowly being cleared of products. Ah.

It looks like the getFilterIds() in very rare cases isn't created or filled properly and may thus contain all ids(!) when a second mass delete action takes place before the last has finished.

So since I still cannot guarantee that my client never makes the mistake of selecting "Select All" from the drop down instead of "Select All on this page" and since I do want to limit the damage in case the delete ends up with all the ids instead of, say, max 200 I decided to go for a quick solution that checks if the array is not empty and does not contain more then x (in my case 200) ids, it can filter and if not it should filter on empty.

My current fix for Magento 2.2.6 in file vendor/magento/module-ui/Component/Massaction/Filter.php in public function getCollection(AbstractDb $collection) is now:

$filterIds = $this->getFilterIds();

if (!empty($filterIds) && (count($filterIds) < 200)) { $collection->addFieldToFilter( $collection->getIdFieldName(), ['in' => $filterIds] ); } else { $collection->addFieldToFilter( $collection->getIdFieldName(), ['in' => ''] ); }

return $collection;

In the above I simply check: is it filled and does it contain less then 200 products, go filter using the provided ids. If not, filter on empty (i.e. do not delete anything). Simply skipping the filter isn't an option as all products will be deleted.

Since this is old and I don't have enough proof (I only reproduced this once!), I don't want to reopen this issue. Looking at the code of the current Magento 2.3 I wonder if it may happen there too. I am thinking of making this into a module in which the customer can set a "maximum number of deletes" maybe under Stores -> Configuration -> Catalog -> Catalog and then maybe a "set max number of products/customers/orders that can be deleted or so. Would it be a good idea if in the next Magento 2.3.x there were such an option?

patrick-bigbridge commented 5 years ago

@SolsWebdesign I suggest that this issue is reopened. We have now seen two cases where the issue still exists (even if if occurs less frequently). This is an important bug that can have a great impact on a webshop.

I myself cannot reopen it since I am not a Magento repo contributor.

kapil-infobeans commented 5 years ago

I agree, we are seeing the issue in Magento EE 2.3.2 version as well. There is no fix for it yet, we removed mass delete for products and make an option for delete at product action level.

hostep commented 5 years ago

Let's reopen this then. Some steps to reproduce - even though I understand this is hard to figure out - would be appreciated! :)

m2-assistant[bot] commented 5 years ago

Hi @engcom-Charlie. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[x] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
Details
If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.
[x] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.
[x] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
[x] 4. Verify that the issue is reproducible on 2.3-develop branch
Details
- Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Add label Issue: Confirmed once verification is complete.
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

magento-engcom-team commented 5 years ago

:white_check_mark: Confirmed by @engcom-Charlie Thank you for verifying the issue. Based on the provided information internal tickets MC-19607 were created

Issue Available: @engcom-Charlie, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

shreyasavion commented 4 years ago

Hello, I got solution. I know that this is not reliable but it works for me. I made changes in Massdelete controller.

$count = count($collection); // newly added foreach ($collection as $product) { $this->productRepository->delete($product); $productDeleted++; if($count == $productDeleted){break;} // newly added }

And now i am going to override this controller. And Looking forward to correct answer.

o-iegorov commented 4 years ago

Core team start working on this issue

magento-engcom-team commented 4 years ago

Hi @patrick-bigbridge.

Thank you for your report and collaboration!

The issue was fixed by Magento team. The fix was delivered into magento/magento2:2.3-develop branch(es). Related commit(s):

The fix will be available with the upcoming 2.3.4 release.

patrick-bigbridge commented 4 years ago

@magento-engcom-team Thanks, but please check the links to the commits. They are broken.

sdzhepa commented 4 years ago

Hello @patrick-bigbridge

By mistake, the internal Jira ticket was closed before code has been delivered into 2.3-develop branch. At this moment we have next status for this issue:

Internal team has fixed this issue
Fix has been code-reviewed and QA passed
Internal Pull Request in the delivery queue now and should be delivered into 2.3-develop branch during the next couple of days

After delivery links from https://github.com/magento/magento2/issues/15935#issuecomment-543987670 should become valid. If not I will provide valid links as a comment after merge

Sorry for possible inconveniences

sdzhepa commented 4 years ago

Hello @patrick-bigbridge

FYI: Internal PR has been merged. Links with commits from https://github.com/magento/magento2/issues/15935#issuecomment-543987670 are available now

patrick-bigbridge commented 4 years ago

Thanks!

amitmaurya191 commented 4 years ago

Hello, I got solution. I know that this is not reliable but it works for me. I made changes in Massdelete controller.

$count = count($collection); // newly added foreach ($collection as $product) { $this->productRepository->delete($product); $productDeleted++; if($count == $productDeleted){break;} // newly added }

And now i am going to override this controller. And Looking forward to correct answer.

did you get answer ?

niners52 commented 4 years ago

This just happened to me again on 2.3.2-p2.

pedrohenrique1986 commented 4 years ago

Hello, this ended up happening to a customer, today 04/15/2020, he deleted 2 products yesterday 04/14/2020, the admin panel crashed, he turned off the computer, today when he logged in the admin panel, all the products disappeared.

You check the logs that occur, and identify several lines of "catalog / product / massDelete".

Magento2 Version: 2.3.3

kapil-infobeans commented 4 years ago

I think until magento fixes this issue, you can remove mass delete action from product grid, mass actions list. And add delete option at product row level.

magento / magento2