-
Notifications
You must be signed in to change notification settings - Fork 131
MQE-1065: Persisted data are not resolved correctly when using with ParameterArray #222
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
…arameterArray - bug fix
…arameterArray - addressed static test failures
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.
Overall looks good and works as expected, just need some documentation updates to clarify reasoning behind some lines.
$arrayResult = []; | ||
$count = 0; | ||
|
||
// matches arrays |
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.
I think we need a little more commenting on here, what's actually happening is more like
find arrays, replace them with placeholder to prevent manipulation in foreach below
preg_match_all('/[\[][^\]]*?[\]]/', $input, $paramInput); | ||
if (!empty($paramInput)) { | ||
foreach ($paramInput[0] as $param) { | ||
$arrayResult[static::PRESSKEY_ARRAY_ANCHOR_KEY . $count] = |
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.
We call class constants as self::CONSTANT_NAME
everywhere else in the framework, may be worth doing the same here.
} | ||
|
||
$result = implode(',', $result); | ||
if (!empty($arrayResult)) { |
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.
Comment here:
Reinsert arrays into result
…arameterArray - addressed review comments
@jilu1 changes are good to go. I changed the title of the PR to conform to normal |
Description
MQE-1065: Persisted data are not resolved correctly when using with ParameterArray
Contribution checklist