Fix/trigger webhook on bulk delete #14041
Conversation
Marc-Roig
added
source: core:strapi
|
About the components not being removed from the database, @derrickmehaffy mentions in the issue that we could modify the foreign key constrain to enable cascade delete. |
Codecov Report
@@ Coverage Diff @@
## master #14041 +/- ##
==========================================
+ Coverage 55.35% 55.46% +0.11%
==========================================
Files 1258 1258
Lines 31699 31708 +9
Branches 5734 5736 +2
==========================================
+ Hits 17546 17588 +42
+ Misses 12337 12307 -30
+ Partials 1816 1813 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| await deleteComponents(uid, entityToDelete); | ||
| await db.query(uid).delete({ where: { id: entityToDelete.id } }); |
There was a problem hiding this comment.
Why did you change the order? It was not working before?
There was a problem hiding this comment.
deleteComponents was querying for the uid entity:
// In deleteComponents:
const value = await strapi.query(uid).load(entityToDelete, attributeName);the entity was already deleted at the time deleteComponents was executed.
| return db.query(uid).deleteMany(query); | ||
| const entitiesToDelete = await db.query(uid).findMany({ | ||
| ...query, | ||
| }); |
There was a problem hiding this comment.
What about:
const entitiesToDelete = await db.query(uid).findMany(query);?
| @@ -161,6 +161,13 @@ describe('Core API - Basic + compo + draftAndPublish', () => { | |||
| data.productsWithCompoAndDP.shift(); | |||
| }); | |||
|
|
|||
| describe('database state', () => { | |||
| test('components have been removed from the database', async () => { | |||
| const dbComponents = await strapi.db.query('default.compo').findMany({}); | |||
There was a problem hiding this comment.
Let's see what the fun test run says but it may not work as the test database may have data left from the other tests
There was a problem hiding this comment.
Isn't the database pruned in each test file?
Maybe I can add a query in the findMany referring to the component added in this test file then.
There was a problem hiding this comment.
I'm not sure, it would be nice to check and know. Js wrote the code
There was a problem hiding this comment.
I will add the query just in case then. And also check if it works first.
|
Very nice! |
Sure! What do you think if we leave the deleteComponents reorder & test as a temporary fix, and we work on the cascade delete in another PR? |
|
@petersg83 From there, I saw a PR made by @stafyniaksacha some months ago: #12653 He changed the order of deletion (between component and entity) to allow developers to query the entity being deleted including the components on the beforeDelete hook. So I think I will leave things as they were, and maybe we can have this in mind when planning the cascade delete. Note: props to @stafyniaksacha for the e2e tests that made me realize this :) |
ππ» What does it do?
β Why is it needed?
It was triggered when deleting a single entity but not for a bulk delete.
π§ͺ How to test it?
You can use this site to test the webhook https://webhook.site/
π Related issue(s)/PR(s)
#12290