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

#13612 Fixed-Quantity_and_stock_status when visibility set to storefront throwing exception #20001

Conversation

aditisinghcedcoss
Copy link
Contributor

@aditisinghcedcoss aditisinghcedcoss commented Dec 28, 2018

Description (*)

The function getOptionText expects params as string or integer and in case of quantity_and_stock_status when visibility set to storefront , array passed to getOptionText which can be handled by adding is_scalar check for param $value.

Fixed Issues (if relevant)

  1. magento/magento2 1 exception(s): Exception #0 (Exception): Warning: Illegal offset type in isset or empty in /home/jewelrynest2/public_html/magento/vendor/magento/module-eav/Model/Entity/Attribute/Source/AbstractSource.php on line 76 #13612: Issue 1 exception(s): Exception #0 (Exception): Warning: Illegal offset type in isset or empty in /Model/Entity/Attribute/Source/AbstractSource.php on line 76

Manual testing scenarios (*)

Contribution checklist (*)

  • [ x] Pull request has a meaningful description of its purpose
  • [ x] 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 on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Dec 28, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @aditisinghcedcoss. 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-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

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

@@ -73,7 +73,7 @@ public function getOptionText($value)
}
}
// End
if (isset($options[$value])) {
if (is_null($value) === false && is_array($value) === false && isset($options) && isset($options[$value])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @aditisinghcedcoss,

Array keys can only be scalar, but $value is an array.

It looks like it's enough to check is_scalar($value). Please update code and do amend commit.

Condition like is_null($value) === false looks really odd and is never a good idea.

Copy link
Contributor Author

@aditisinghcedcoss aditisinghcedcoss Dec 28, 2018

Choose a reason for hiding this comment

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

@orlangur sir,

i have made the amendments.

Thank you.

Copy link
Contributor

Choose a reason for hiding this comment

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

@aditisinghcedcoss, only those checks are needed:

is_scalar($value) && isset($options[$value])

Please squash changes into a single commit and do a force push.

Copy link
Contributor Author

@aditisinghcedcoss aditisinghcedcoss Jan 2, 2019

Choose a reason for hiding this comment

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

@orlangur sir,

I have squashed changes into a single commit and forced pushed.

Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@orlangur sir,

Please Review.

Thank you

@aditisinghcedcoss aditisinghcedcoss force-pushed the aditisinghcedcoss-patch-for-issue#13612 branch from 8dc2060 to 0df7243 Compare January 2, 2019 06:49
@magento-engcom-team magento-engcom-team added this to the Release: 2.3.1 milestone Jan 9, 2019
@magento-engcom-team
Copy link
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-3810 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@aditisinghcedcoss thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@irajneeshgupta irajneeshgupta changed the title Issue #13612 Fixed. #13612 Fixed-Quantity_and_stock_status when visibility set to storefront throwing exception Jan 15, 2019
@ghost
Copy link

ghost commented Mar 16, 2019

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

@meker12
Copy link
Contributor

meker12 commented Jun 3, 2019

Draft release note:

Infrastructure

Added an is_array parameter value check to the getOptionText($value) function in app/code/Magento/Eav/Model/Entity/Attribute/Source/AbstractSource.php to fix an error that caused Magento to throw a UI error when you change the quantity_and_stock_status (Qty) product attribute visibility settings in the Storefront properties. Previously, the getOptionText($value)` function expected integer or string input and failed if parameters were passed as an array.

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

Successfully merging this pull request may close these issues.

8 participants