Check if users before attempting to access #66
Conversation
|
@ckrazoyo what version are you running now? master from Github or a specific version from Magento connect? |
|
Hello, Thanks, On Tue, May 19, 2015 at 11:38 AM, Jason Smale notifications@github.com
|
|
Hi, Thanks, On Wed, May 20, 2015 at 4:20 PM, Chris Kendall ckendall@razoyo.com wrote:
|
|
Thanks for providing the PR. We'll be reviewing it this week. |
|
👍 we can bundle this with the next release. I see nothing wrong with the code, quite similar to our previous fixes. Just want to note that the extension has only been tested up to Enterprise Edition 1.12 . |
|
|
||
| if( count($found) ) { | ||
|
|
||
| if ($users) { |
There was a problem hiding this comment.
Can we Just enclose the whole thing in the if block to save on checks?
if ($users) {
$found = array_filter($users, function($user) use($value) {
return (int) $user['id'] === $value;
});
if( count($found) ) {
$user = array_shift($found);
return $user['email'];
}
}
return '';
}
Also can we remove the whitespaces at the end of lines 37, 39, 40
There was a problem hiding this comment.
As closure functions are supported from PHP >= 5.3.0, I would discourage to use them within Magento code as they are not used anywhere else. It does make use of create_function() or you could just simply iterate over the array with foreach and return $user['email'] when the right user ID was found.
Check if users before attempting to access
|
Thanks @ckrazoyo |
Updated previously existing Magento Zendesk extension to this version, afterwords Magento admin panel threw following error. Resolved by wrapping statement in if(users) to ensure there are users before attempting to continue.
Client on EE Magento ver. 1.13.1.0