-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Backward compatibility provided by #19765 #23407
Conversation
Hi @Nazar65. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
@magento give me 2.3-develop instance |
Hi @Dren7755. Thank you for your request. I'm working on Magento 2.3-develop instance for you |
Hi @Dren7755, here is your Magento instance. |
@@ -408,8 +416,8 @@ protected function _saveValidatedBunches() | |||
if ($source->valid()) { | |||
try { | |||
$rowData = $source->current(); | |||
if (array_key_exists('sku', $rowData)) { | |||
$skuSet[$rowData['sku']] = true; | |||
if ($this->uniqueField !== null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, revert array_key_exists check. We'll have undefined index error if $rowData does not contain $this->uniqueField field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@Nazar65 , I am closing this PR now due to inactivity. |
Hi @Nazar65, thank you for your contribution! |
Description (*)
Current fixes still don't provide complete backward compatibility with code designed for Magento 2.0.X, 2.1.X, 2.2.X.
IMO it is not good at all to have any variables related to SKU in abstract entity code. In the best case new class must be implemented and extended from abstract entity.
In case it is impossible to achieve that, then it would be probably better to introduce one more variable to the constructor.
Fixed Issues (if relevant)
Manual testing scenarios (*)
(override 'uniqueField' to your unique field)
Contribution checklist (*)