Bundle products options input validation #29256

michalderlatka · 2020-07-24T10:10:44Z

Description (*)

Validates input for radio and select type options.

Related Pull Requests

Fixed Issues (if relevant)

Fixes Bundle product with option type radio and dropdown can be added to the cart with multiple choices #26110

Manual testing scenarios (*)

Follow instructions from related issue

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)

m2-assistant · 2020-07-24T10:10:46Z

Hi @michalderlatka. 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.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

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

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

michalderlatka · 2020-07-24T10:10:57Z

@magento run all tests

michalderlatka · 2020-07-24T11:27:10Z

@magento run all tests

cpartica · 2020-07-24T18:40:56Z

app/code/Magento/Bundle/Model/Product/Type.php

+     * @return void
+     * @throws \Magento\Framework\Exception\LocalizedException
+     */
+    protected function validateRadioAndSelectOptions($optionsCollection, $options): void


don't add protected, use private

cpartica · 2020-07-24T18:42:31Z

app/code/Magento/Bundle/Model/Product/Type.php

+
+        if (is_array($optionsCollection->getItems())) {
+            foreach ($optionsCollection->getItems() as $option) {
+                if (($option->getType() == 'radio' || $option->getType() == 'select') &&


that is a big condition, can you move that to a method with a meaningful name?

cpartica · 2020-07-24T18:43:00Z

app/code/Magento/Bundle/Model/Product/Type.php

+    /**
+     * Validate Options for Radio and Select input types
+     *
+     * @param \Magento\Bundle\Model\ResourceModel\Option\Collection $optionsCollection


do "use Magento\Bundle\Model\ResourceModel\Option\Collection" and only use "Collection"

app/code/Magento/Bundle/Model/Product/Type.php

lbajsarowicz · 2020-07-25T12:36:23Z

Hi @michalderlatka . Nice to see another contributor from Poland.
Please review the Maintainer comments and feel free to share your thoughts.

michalderlatka · 2020-07-30T10:40:21Z

@magento run all tests

michalderlatka · 2020-08-03T05:19:12Z

@magento run static tests

cpartica · 2020-08-03T17:23:28Z

@magento run all tests

cpartica

Some static fails, but also some possible performance problems:

PHP Code Sniffer detected 2 violation(s):

FILE: /var/www/html/app/code/Magento/Bundle/Model/Product/Type.php

FOUND 1 ERROR AND 4 WARNINGS AFFECTING 5 LINES

398 | WARNING | [ ] Function's nesting level (6) exceeds 5;
| | consider refactoring the function
535 | ERROR | [x] Tags visual alignment must be consistent
1194 | WARNING | [ ] Empty FUNCTION statement detected
1212 | WARNING | [ ] array_merge(...) is used in a loop and is a
| | resources greedy construction.
1393 | WARNING | [ ] array_merge(...) is used in a loop and is a
| | resources greedy construction.

PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY

The following files incorrectly reuse fixtures:
/var/www/html/dev/tests/integration/testsuite/Magento/Bundle/_files/product_with_multiple_options_radio_select.php
/var/www/html/dev/tests/integration/testsuite/Magento/Bundle/_files/product_with_multiple_options_radio_select_rollback.php
Please use Magento\TestFramework\Workaround\Override\Fixture\Resolver::requireDataFixture
Failed asserting that 2 matches expected 0.

cpartica · 2020-08-03T18:36:13Z

app/code/Magento/Bundle/Model/Product/Type.php

@@ -459,12 +461,12 @@ public function getOptionsIds($product)
     * Retrieve bundle option collection
     *
     * @param \Magento\Catalog\Model\Product $product
-     * @return \Magento\Bundle\Model\ResourceModel\Option\Collection


SVC is failing because of this, but I will approve it, it's a false positive

cpartica · 2020-08-03T18:36:26Z

app/code/Magento/Bundle/Model/Product/Type.php

@@ -887,7 +894,7 @@ public function getSelectionsByIds($selectionIds, $product)
     *
     * @param array $optionIds
     * @param \Magento\Catalog\Model\Product $product
-     * @return \Magento\Bundle\Model\ResourceModel\Option\Collection


SVC is failing because of this, but I will approve it, it's a false positive

…erlatka/magento2 into 26110_bundle_product_options_fix

michalderlatka · 2020-08-05T12:54:58Z

@magento run all tests

michalderlatka · 2020-08-05T18:57:03Z

@magento run Functional Tests B2B

cpartica · 2020-08-10T20:49:42Z

@magento run Functional Tests B2B

cpartica · 2020-08-10T20:49:47Z

@magento run Functional Tests EE

m2-assistant · 2020-08-11T19:33:04Z

Hi @michalderlatka, 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.

ghost added the Progress: pending review label Jul 24, 2020

magento-engcom-team added Component: Bundle Release Line: 2.4 labels Jul 24, 2020

michalderlatka added PAP Partners acceleration program Project: GraphQL labels Jul 24, 2020

michalderlatka force-pushed the 26110_bundle_product_options_fix branch from 59d831a to 71bdba5 Compare July 24, 2020 11:26

cpartica requested changes Jul 24, 2020

View reviewed changes

ghost assigned cpartica Jul 24, 2020

ghost added Progress: needs update and removed Progress: pending review labels Jul 24, 2020

magento#26110: Bundle products options input validation

28e20d9

michalderlatka force-pushed the 26110_bundle_product_options_fix branch from 71bdba5 to 28e20d9 Compare July 30, 2020 10:39

cpartica approved these changes Aug 3, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Aug 3, 2020

Merge branch '2.4-develop' into 26110_bundle_product_options_fix

bbb330f

cpartica requested changes Aug 3, 2020

View reviewed changes

ghost added Progress: needs update and removed Progress: ready for testing labels Aug 3, 2020

cpartica reviewed Aug 3, 2020

View reviewed changes

Michal Derlatka added 2 commits August 5, 2020 14:53

magento#26110: Bundle products options input validation

101a121

Merge branch '26110_bundle_product_options_fix' of github.com:michald…

1c18a1e

…erlatka/magento2 into 26110_bundle_product_options_fix

michalderlatka requested a review from cpartica August 7, 2020 14:26

cpartica approved these changes Aug 10, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Aug 10, 2020

Merge branch '2.4-develop' into 26110_bundle_product_options_fix

26076a1

ghost added Progress: testing in progress Progress: accept and removed Progress: ready for testing Progress: testing in progress labels Aug 10, 2020

Static test fixes

9f9e98a

magento-engcom-team merged commit 5e832f3 into magento:2.4-develop Aug 11, 2020

ihor-sviziev mentioned this pull request Sep 25, 2020

Implemented Option Type validation in validationItemTypes #26122

Closed

Bundle products options input validation #29256

Bundle products options input validation #29256

Uh oh!

Conversation

michalderlatka commented Jul 24, 2020

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Jul 24, 2020

Uh oh!

michalderlatka commented Jul 24, 2020

Uh oh!

michalderlatka commented Jul 24, 2020

Uh oh!

cpartica Jul 24, 2020

Choose a reason for hiding this comment

Uh oh!

cpartica Jul 24, 2020

Choose a reason for hiding this comment

Uh oh!

cpartica Jul 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lbajsarowicz commented Jul 25, 2020

Uh oh!

michalderlatka commented Jul 30, 2020

Uh oh!

michalderlatka commented Aug 3, 2020

Uh oh!

cpartica commented Aug 3, 2020

Uh oh!

cpartica left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

FILE: /var/www/html/app/code/Magento/Bundle/Model/Product/Type.php

FOUND 1 ERROR AND 4 WARNINGS AFFECTING 5 LINES

PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY

Uh oh!

cpartica Aug 3, 2020

Choose a reason for hiding this comment

Uh oh!

cpartica Aug 3, 2020

Choose a reason for hiding this comment

Uh oh!

michalderlatka commented Aug 5, 2020

Uh oh!

michalderlatka commented Aug 5, 2020

Uh oh!

cpartica commented Aug 10, 2020

Uh oh!

cpartica commented Aug 10, 2020

Uh oh!

m2-assistant bot commented Aug 11, 2020

Uh oh!

Uh oh!

cpartica left a comment •

edited

Loading