Check if the product exists in the system before adding to comparing list. #23073

adarshkhatri · 2019-05-31T01:24:29Z

Check if the product exists in the system before adding to comparing list.

Description (*)

Check if the product exists in the system before adding to comparing list. If the product doesn't exist, then we simply do nothing.

Fixed Issues (if relevant)

Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails #23071: Integrity constraint violation: 1452 Cannot add or update a child row: a foreign key constraint fails

Manual testing scenarios (*)

Use this url http:://www.domain.com/catalog/product_compare/index/items/ab
Use this url http:://www.domain.com/catalog/product_compare/index/items/1234567890
Make sure product ID doesn't exist in the system.

Questions or comments

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

Check if the product exists in the system before adding to comparing list. If the product doesn't exist, then we simply do nothing.

m2-assistant · 2019-05-31T01:24:31Z

Hi @adarshkhatri. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

adarshkhatri · 2019-05-31T02:18:19Z

@magento give me test instance

magento-engcom-team · 2019-05-31T02:18:24Z

Hi @adarshkhatri. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-05-31T02:19:11Z

Hi @adarshkhatri, here is your new Magento instance.
Admin access: https://pr-23073.instances.magento-community.engineering/admin
Login: admin Password: 123123q

app/code/Magento/Catalog/Model/Product/Compare/ListCompare.php

arnoudhgz · 2019-06-01T11:05:10Z

@adarshkhatri @dmytro-ch Is it not better to check if the product exists as first thing in the addProduct method? It does not make sense to do anything else (like _addVisitorToItem and loadByProduct) in this method if the product does not exist.

orlangur

After changes are applied and all builds are green, please squash them into a single commit so that we have perfectly clean history 😉

app/code/Magento/Catalog/Model/Product/Compare/ListCompare.php

Using searchCriteriaBuilder to get the products from the system, filtered by (passed) productIds and then iterate that list of products to add to compare list.

adarshkhatri · 2019-06-06T00:15:08Z

@magento give me test instance

magento-engcom-team · 2019-06-06T00:15:09Z

Hi @adarshkhatri. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-06-06T00:16:22Z

Hi @adarshkhatri, here is your new Magento instance.
Admin access: https://pr-23073.instances.magento-community.engineering/admin
Login: admin Password: 123123q

orlangur

After changes are applied and all builds are green, please squash them into a single commit so that we have perfectly clean history 😉

orlangur · 2019-06-06T13:27:27Z

app/code/Magento/Catalog/Model/Product/Compare/ListCompare.php

@@ -51,6 +51,18 @@ class ListCompare extends \Magento\Framework\DataObject
     */
    protected $_compareItemFactory;

+    /**
+     * Product Repository


Such comment is useless, please remove.

app/code/Magento/Catalog/Model/Product/Compare/ListCompare.php

Adding backward compatibility for constructor parameters.

Fixing typo "_condProdCombineF". Replacing it with "_condProdCombineFactory".

app/code/Magento/SalesRule/Model/Rule.php

m2-assistant · 2019-08-27T06:37:20Z

Hi @adarshkhatri, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Currently, the system tries to add any product id in to compare list, which is why it fails and throws an error. Allow only existing products to be added to compare list.

update to recent version

m2-assistant · 2019-09-06T00:35:48Z

Hi @adarshkhatri. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

adarshkhatri · 2019-09-06T00:43:08Z

@magento give me test instance

magento-engcom-team · 2019-09-06T00:43:13Z

Hi @adarshkhatri. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-09-06T00:43:51Z

Hi @adarshkhatri, here is your new Magento instance.
Admin access: https://pr-23073.instances.magento-community.engineering/admin
Login: admin Password: 123123q

sidolov · 2019-09-23T16:15:02Z

@adarshkhatri please, cover changes with automated tests

adarshkhatri · 2019-09-23T23:09:15Z

@adarshkhatri please, cover changes with automated tests

I Couldn't understand them.

First fail:

PHP Code Mess has found error(s):
/var/www/html/app/code/Magento/Catalog/Model/Product/Compare/ListCompare.php:20 The class Magento\Catalog\Model\Product\Compare\ListCompare uses sessions or cookies while not being a part of HTML Presentation layer

Failed asserting that 2 matches expected 0.

Second fail:

Database Compare / Database Compare build failing 12 days ago in 30m 38s
No build reports available

sidolov · 2019-09-27T19:36:37Z

@adarshkhatri test failures caused by legacy implementation and you don't need to fix them. Please, cover newly added logic by tests

adarshkhatri · 2019-09-30T01:50:42Z

@adarshkhatri test failures caused by legacy implementation and you don't need to fix them. Please, cover newly added logic by tests

Just couldn't understand what's the logic added by test in this?

Should I just add @SuppressWarnings(PHPMD.CookieAndSessionMisuse) in the class?

Thanks.

sidolov · 2019-10-04T15:15:20Z

@adarshkhatri usage of the repository was added to the method, it should be covered at least by unit test.

dmytro-ch · 2019-10-23T15:08:37Z

Closing this PR due to inactivity.
Feel free to reopen this PR if you wish to proceed.
Thank you!

m2-assistant · 2019-10-23T15:08:39Z

Hi @adarshkhatri, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Check if product exists in the system before adding to comparing list.

b4114dd

Check if the product exists in the system before adding to comparing list. If the product doesn't exist, then we simply do nothing.

magento-engcom-team added Component: Catalog Release Line: 2.3 labels May 31, 2019

adarshkhatri added 3 commits May 31, 2019 11:33

Update ListCompare.php

f94592b

Update ListCompare.php

7168a4b

Update ListCompare.php

e0f9536

dmytro-ch self-assigned this May 31, 2019

dmytro-ch self-requested a review May 31, 2019 05:33

dmytro-ch requested changes May 31, 2019

View reviewed changes

ghost added the Progress: needs update label May 31, 2019

Update ListCompare.php

89568b5

adarshkhatri requested a review from dmytro-ch June 4, 2019 00:41

orlangur requested changes Jun 5, 2019

View reviewed changes

app/code/Magento/Catalog/Model/Product/Compare/ListCompare.php Outdated Show resolved Hide resolved

app/code/Magento/Catalog/Model/Product/Compare/ListCompare.php Outdated Show resolved Hide resolved

ghost assigned orlangur Jun 5, 2019

adarshkhatri added 2 commits June 6, 2019 10:09

Update ListCompare.php

90be167

Using searchCriteriaBuilder to get the products from the system, filtered by (passed) productIds and then iterate that list of products to add to compare list.

Update ListCompare.php

22b5669

adarshkhatri requested a review from orlangur June 6, 2019 01:08

orlangur requested changes Jun 6, 2019

View reviewed changes

Adding backward compatibility

937f24b

Adding backward compatibility for constructor parameters.

adarshkhatri requested a review from orlangur June 7, 2019 00:57

Fixing typo "_condProdCombineF"

504e8fc

Fixing typo "_condProdCombineF". Replacing it with "_condProdCombineFactory".

sidolov requested changes Aug 16, 2019

View reviewed changes

app/code/Magento/SalesRule/Model/Rule.php Outdated Show resolved Hide resolved

ghost assigned sidolov Aug 16, 2019

Update Rule.php

8ef2147

adarshkhatri closed this Aug 27, 2019

adarshkhatri added 3 commits August 27, 2019 16:45

Only existing products to be added to compare list

a13e537

Currently, the system tries to add any product id in to compare list, which is why it fails and throws an error. Allow only existing products to be added to compare list.

Merge pull request #1 from magento/2.3-develop

8fdad46

update to recent version

Removing extra comment.

4edd03c

adarshkhatri reopened this Sep 6, 2019

adarshkhatri requested a review from akaplya as a code owner September 6, 2019 00:35

ghost unassigned orlangur, sidolov and dmytro-ch Sep 6, 2019

Removed php mess.

df5be91

dmytro-ch assigned dmytro-ch, orlangur and sidolov Oct 16, 2019

dmytro-ch closed this Oct 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check if the product exists in the system before adding to comparing list. #23073

Check if the product exists in the system before adding to comparing list. #23073

adarshkhatri commented May 31, 2019 •

edited

Loading

m2-assistant bot commented May 31, 2019

adarshkhatri commented May 31, 2019

magento-engcom-team commented May 31, 2019

magento-engcom-team commented May 31, 2019

arnoudhgz commented Jun 1, 2019

orlangur left a comment

adarshkhatri commented Jun 6, 2019

magento-engcom-team commented Jun 6, 2019

magento-engcom-team commented Jun 6, 2019

orlangur left a comment

orlangur Jun 6, 2019

adarshkhatri Sep 6, 2019

m2-assistant bot commented Aug 27, 2019

m2-assistant bot commented Sep 6, 2019

adarshkhatri commented Sep 6, 2019

magento-engcom-team commented Sep 6, 2019

magento-engcom-team commented Sep 6, 2019

sidolov commented Sep 23, 2019

adarshkhatri commented Sep 23, 2019

sidolov commented Sep 27, 2019

adarshkhatri commented Sep 30, 2019

sidolov commented Oct 4, 2019

dmytro-ch commented Oct 23, 2019 •

edited

Loading

m2-assistant bot commented Oct 23, 2019

Check if the product exists in the system before adding to comparing list. #23073

Check if the product exists in the system before adding to comparing list. #23073

Conversation

adarshkhatri commented May 31, 2019 • edited Loading

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented May 31, 2019

adarshkhatri commented May 31, 2019

magento-engcom-team commented May 31, 2019

magento-engcom-team commented May 31, 2019

arnoudhgz commented Jun 1, 2019

orlangur left a comment

Choose a reason for hiding this comment

adarshkhatri commented Jun 6, 2019

magento-engcom-team commented Jun 6, 2019

magento-engcom-team commented Jun 6, 2019

orlangur left a comment

Choose a reason for hiding this comment

orlangur Jun 6, 2019

Choose a reason for hiding this comment

adarshkhatri Sep 6, 2019

Choose a reason for hiding this comment

m2-assistant bot commented Aug 27, 2019

m2-assistant bot commented Sep 6, 2019

adarshkhatri commented Sep 6, 2019

magento-engcom-team commented Sep 6, 2019

magento-engcom-team commented Sep 6, 2019

sidolov commented Sep 23, 2019

adarshkhatri commented Sep 23, 2019

sidolov commented Sep 27, 2019

adarshkhatri commented Sep 30, 2019

sidolov commented Oct 4, 2019

dmytro-ch commented Oct 23, 2019 • edited Loading

m2-assistant bot commented Oct 23, 2019

adarshkhatri commented May 31, 2019 •

edited

Loading

dmytro-ch commented Oct 23, 2019 •

edited

Loading