verbb / wishlist

A Craft CMS plugin for wishlists for your users to save things to
Other
11 stars 12 forks source link

Items are not deleted when purging inactive lists #106

Closed gddotorg closed 2 years ago

gddotorg commented 2 years ago

Describe the bug

We found that our _elements and _content tables have a lot of rows (over 2.8 million). So we did some research and found 2.3 million rows of the type verbb\wishlist\elements\Item. However, there are only about 6000 lists with 1 - 15 items each. When running the console command wishlist/lists/purge-inactive-lists, both the lists and the search index are deleted, but not the associated items. Could it be that a function like deleteItemsForList is missing when purging lists?

If I miss something or if you need further information, let me know.

Best regards, Georg

Steps to reproduce

  1. Have an inactive list
  2. Running wishlist/lists/purge-inactive-lists to purge all inactive lists
  3. Have a look in the database (table: elements). List is gone but items are still there.

Craft CMS version

3.7.38

Plugin version

1.4.12

Multi-site?

No

Additional context

No response

engram-design commented 2 years ago

Hmm, we shouldn't need to, due to foreign key constraints, as an item cannot exist without a list. However, looks like you're right that the connected element row still exists, just the wishlist_items has been dropped.

I've also added a new console command to help cleanup these items. Run wishlist/items/cleanup-orphaned-items.

To get this fix early, change your verbb/wishlist requirement in composer.json to:

"require": {
  "verbb/wishlist": "dev-craft-3 as 1.4.12",
  "...": "..."
}

Then run composer update.

gddotorg commented 2 years ago

Many thanks for your quick reply. I have executed the command and it worked perfectly for me locally. I have one more addition: We also have this issue in the content table. So I modified your code on a trial basis: In src/console/controllers/ItemsController.php

Db::delete('{{%content}}', ['elementId' => $itemElement]);
Db::delete('{{%elements}}', ['id' => $itemElement]);

and src/services/Lists.php

 Db::delete('{{%content}}', ['elementId' => $itemIds]);
 Db::delete('{{%elements}}', ['id' => $itemIds]);

After that I did another test run and it worked for me.

engram-design commented 2 years ago

That's strange, dropping the element should remove it from the content table due to foreign key constraints - at least that's what happens for me! It should also remove a row from elements_sites.

gddotorg commented 2 years ago

Hm that's strange. I tested it again without my modification. The number of rows in elements went down from 2.8 million to around 700k. So that is fine. In content and elements_sites however the number of rows does not change (2.0 and 2.4 million rows).

engram-design commented 2 years ago

Hmm, I wonder if there's something wrong your with installs' foreign key configuration? I've just testing adding a new item to a wishlist, and manually deleted it from the elements table, and it'll propagate that change to other tables according to the foreign key rules

gddotorg commented 2 years ago

Ok, it seems to work for me now. The foreign key configuration was correct. So I have now simply re-imported the dump and now all relationships in content and elements_sites are also deleted. Thanks a lot.

gddotorg commented 2 years ago

@engram-design I wanted to ask if you can release a new patched version. Again thank you a lot.

engram-design commented 2 years ago

Sorted in 1.4.14