Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use on delete cascade on foreign keys #199

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

lmeyer1
Copy link

@lmeyer1 lmeyer1 commented Feb 29, 2024

Questions Answers
Description? The foreign keys need on delete cascade in order to allow deletion of a product comment.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#35523.
How to test? Run php bin/console doctrine:schema:update --dump-sql and compare the output. Apply both versions of the SQL script to the DB and test for each the deletion of a product comment in the BO

@@ -37,7 +37,7 @@ class ProductCommentGrade
/**
* @ORM\Id
* @ORM\ManyToOne(targetEntity="ProductComment")
* @ORM\JoinColumn(name="id_product_comment", referencedColumnName="id_product_comment")
* @ORM\JoinColumn(name="id_product_comment", referencedColumnName="id_product_comment", onDelete="CASCADE")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -37,7 +37,7 @@ class ProductCommentReport
/**
* @ORM\Id
* @ORM\ManyToOne(targetEntity="ProductComment")
* @ORM\JoinColumn(name="id_product_comment", referencedColumnName="id_product_comment")
* @ORM\JoinColumn(name="id_product_comment", referencedColumnName="id_product_comment", onDelete="CASCADE")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to Grade

@@ -37,7 +37,7 @@ class ProductCommentUsefulness
/**
* @ORM\Id
* @ORM\ManyToOne(targetEntity="ProductComment")
* @ORM\JoinColumn(name="id_product_comment", referencedColumnName="id_product_comment")
* @ORM\JoinColumn(name="id_product_comment", referencedColumnName="id_product_comment", onDelete="CASCADE")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to Grade

Copy link
Contributor

@leemyongpakva leemyongpakva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'd better remove Manual delete after Switching to Doctrine cascade delete.

@lmeyer1
Copy link
Author

lmeyer1 commented Mar 1, 2024

@leemyongpakva What do you mean by manual delete? In the BO, there is only an option to delete a comment, I didn't find an option to manually delete grades, usefulnesses and reports. Anyway, that would be to complicated.

A working delete button for product comments is a must!

@leemyongpakva
Copy link
Contributor

leemyongpakva commented Mar 1, 2024

@leemyongpakva The option to delete a comment in BO will call this snippet https://github.com/PrestaShop/productcomments/blob/dev/productcomments.php#L210-L218, that in turn call https://github.com/PrestaShop/productcomments/blob/dev/src/Repository/ProductCommentRepository.php#L117-L119
Before this PR, this module uses above 3 lines to manually delete ProductComment's grades, reports and usefulness.

@lmeyer1
Copy link
Author

lmeyer1 commented Mar 1, 2024

@leemyongpakva I see.
Are you sure the code in https://github.com/PrestaShop/productcomments/blob/dev/src/Repository/ProductCommentRepository.php#L117-L119 works?
There the related entities are deleted after $this->remove($entity, true);. Remove the comment will fail if there are related entities present.

Are you sure, the FK are present in your DB ? When I create them in my DB, deletion of a comment no longer works in the BO, unless I also add on delete cascade.

@leemyongpakva
Copy link
Contributor

leemyongpakva commented Mar 1, 2024

@lmeyer1 I have just tried deleting a product comment on a PS 8.1.2 site with ProductComment 6.0.2, and the related grades, reports and usefulness are deleted as well. There is no error at all.
In fact, those 3 Manual delete functions can run without FK, they just need $id_product_comment. That's why I called them Manual.

@lmeyer1
Copy link
Author

lmeyer1 commented Mar 1, 2024

@leemyongpakva
Are you sure, the FK are present in your DB ?

What is the output of php bin/console doctrine:schema:update --dump-sql on your site ? Can you show it?

@leemyongpakva
Copy link
Contributor

leemyongpakva commented Mar 1, 2024

Sure, there it is

CRITICAL [console] Error thrown while running command "doctrine:schema:update --dump-sql". Message: "Warning: filemtime(): stat failed for /var/www/html/PrestaShop/vendor/prestashop/autoload/src/Autoloader.php(113) : eval()'d code" ["exception" => ErrorException { …},"command" => "doctrine:schema:update --dump-sql","message" => "Warning: filemtime(): stat failed for /var/www/html/PrestaShop/vendor/prestashop/autoload/src/Autoloader.php(113) : eval()'d code"]

It looks like I haven't got a chance to break my database myself ;) Let's wait other devs checking.

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