Skip to content

Delete the rows referencing a comment before the comment - #245

Open
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/delete-comment-children-first
Open

Delete the rows referencing a comment before the comment#245
boo-code wants to merge 1 commit into
PrestaShop:devfrom
boo-code:fix/delete-comment-children-first

Conversation

@boo-code

Copy link
Copy Markdown
Questions Answers
Description? Deleting a product comment removes the comment first and its grades, reports and usefulness rows afterwards. Those rows reference the comment, so on a shop that applied the schema this module declares - the entities map id_product_comment as a ManyToOne - the database refuses the first statement with Cannot delete or update a parent row and the comment cannot be deleted from the back office. Removing the referencing rows before the comment works with or without the constraints.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#35523.
How to test? See below.

How to test

The constraints are the ones doctrine:schema:update --dump-sql proposes from this module's own entities, for example:

ALTER TABLE ps_product_comment_grade
  ADD CONSTRAINT FK_A0128571ACF38A54 FOREIGN KEY (id_product_comment) REFERENCES ps_product_comment (id_product_comment);

With that constraint in place, on MySQL 8.4:

DELETE FROM ps_product_comment WHERE id_product_comment = X;    -- comment first
ERROR 1451 (23000): Cannot delete or update a parent row: a foreign key constraint fails

DELETE FROM ps_product_comment_grade WHERE id_product_comment = X;   -- children first
DELETE FROM ps_product_comment WHERE id_product_comment = X;
ok

In the back office: with the constraints applied, deleting a comment from Catalog > Product comments fails before this change and succeeds after.

Why ordering rather than cascade

#199 proposed adding onDelete="CASCADE" to the join columns. That works too, but it only takes effect once each shop regenerates its schema, so shops that already have the constraints keep failing until they do, and it leaves the manual deletes doing nothing.

Reordering needs no schema change, no migration and no assumption about whether a given shop has the constraints at all: the rows are removed explicitly in both cases, and the parent goes last. Shops without the constraints are unaffected, since the same three statements run either way.

@ps-jarvis

Copy link
Copy Markdown

Hello @boo-code!

This is your first pull request on productcomments repository of the PrestaShop project.

Thank you, and welcome to this Open Source community!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

productcomments: Incorrect declaration of foreign keys in Doctrine ORM Entity annotations

2 participants