-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Check variable existence in prepareOptionIds(array) in EavAttribute.php #11728
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
Conversation
The existence of $optionsArray['delete'] does not guarantee the existence of $optionsArray['delete'][$optionId]
@@ -174,7 +174,7 @@ protected function prepareOptionIds(array $optionsArray) | |||
{ | |||
if (isset($optionsArray['value']) && is_array($optionsArray['value'])) { | |||
foreach (array_keys($optionsArray['value']) as $optionId) { | |||
if (isset($optionsArray['delete']) && $optionsArray['delete'][$optionId] == 1) { | |||
if (isset($optionsArray['delete'][$optionId]) && $optionsArray['delete'][$optionId] == 1) { |
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.
Php 500 Error when $optionsArray['delete'][$optionId] is not set
This is just a notice, 500 will be displayed only in developer mode.
Can $optionsArray['delete'][$optionId]
contain anything but 1
? Why not do unconditional unset($optionsArray['value'][$optionId])
?
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.
Simply misunderstood! Read the function definition comment: "Get options array without deleted items"
so... not all array_keys($optionsArray['value']) are present in $optionsArray['delete'][$optionId] as indexes of the $optionsArray['delete']
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.
Yeah, I've got the point of bugfix, just trying to simplify the code.
Is check for == 1
essential or we can simply do unset
without condition?
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.
At first glance I would say yes, but checking getData() method of Attribute I can't be totally sure.
Anyway a condition must be verified, so the choice should be between:
(isset($optionsArray['delete'][$optionId]))
and
(isset($optionsArray['delete'][$optionId]) && $optionsArray['delete'][$optionId] == 1)
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, it is already merged, I was trying to understand if it can be just if (!empty($optionsArray['delete'][$optionId]))
.
Description
The existence of $optionsArray['delete'] does not guarantee the existence of $optionsArray['delete'][$optionId]
Fixed Issues (if relevant)