-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: simplify input handling and make it consistent #4894
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
base: master
Are you sure you want to change the base?
Changes from all commits
9782cdc
dbea51b
f782e03
f8f0cdf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -181,85 +181,47 @@ public function setInput(array $input) | |||||||||
| private function setInputWithContext(array $input, $queriedContext) | ||||||||||
| { | ||||||||||
| $parameters = $this->getParameters(); | ||||||||||
|
|
||||||||||
| // Import and assign all inputs to their context | ||||||||||
| foreach ($input as $name => $value) { | ||||||||||
| foreach ($parameters as $context => $set) { | ||||||||||
| if (array_key_exists($name, $parameters[$context])) { | ||||||||||
| $this->inputs[$context][$name]['value'] = $value; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| if (!array_key_exists($queriedContext, $parameters)) { | ||||||||||
| // unknown context provided by client, throw exception here? or continue? | ||||||||||
|
papaj-na-wrotkach marked this conversation as resolved.
|
||||||||||
| } | ||||||||||
|
|
||||||||||
| // Apply default values to missing data | ||||||||||
| $contextNames = [$queriedContext]; | ||||||||||
| if (array_key_exists('global', $parameters)) { | ||||||||||
| $contextNames[] = 'global'; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| foreach ($contextNames as $context) { | ||||||||||
| if (!isset($parameters[$context])) { | ||||||||||
| // unknown context provided by client, throw exception here? or continue? | ||||||||||
| } | ||||||||||
|
|
||||||||||
| foreach ($parameters[$context] as $name => $parameter) { | ||||||||||
| if (isset($this->inputs[$context][$name]['value'])) { | ||||||||||
| // Skip this property if we already set the value when iterating through another context. | ||||||||||
| if (isset($this->inputs[$queriedContext][$name]['value'])) { | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| $type = $parameter['type'] ?? 'text'; | ||||||||||
|
|
||||||||||
| switch ($type) { | ||||||||||
| // `… ?? null` protects us from 'Undefined array key' errors. It fallbacks to null if the value is null or the key does not exist. | ||||||||||
| $value = $input[$name] ?? null; | ||||||||||
| switch ($parameter['type'] ?? 'text') { | ||||||||||
| case 'checkbox': | ||||||||||
| $this->inputs[$context][$name]['value'] = $input[$context][$name]['value'] ?? false; | ||||||||||
| // We don't use default value. Use the value if value is set, `false` if not. | ||||||||||
| $value ??= false; | ||||||||||
|
Comment on lines
+202
to
+203
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could make checkbox values boolean only, not mixed (value or I did not implement it yet because it could be a breaking change if some bridge relied on this value being set to something specific instead of relying on it being not false (or not falsy/empty). Let me know if you want me to change it.
Suggested change
|
||||||||||
| break; | ||||||||||
| case 'list': | ||||||||||
| if (!isset($parameter['defaultValue'])) { | ||||||||||
| $firstItem = reset($parameter['values']); | ||||||||||
| if (is_array($firstItem)) { | ||||||||||
| $firstItem = reset($firstItem); | ||||||||||
| } | ||||||||||
| $this->inputs[$context][$name]['value'] = $firstItem; | ||||||||||
| } else { | ||||||||||
| $this->inputs[$context][$name]['value'] = $parameter['defaultValue']; | ||||||||||
| $value ??= ($parameter['defaultValue'] ?? null); | ||||||||||
| // Break from the switch statement if we set the value or default value was not null. | ||||||||||
| if ($value !== null) { | ||||||||||
| break; | ||||||||||
| } | ||||||||||
| // Get the first item or its first subitem. | ||||||||||
| $value = reset($parameter['values']); | ||||||||||
| if (is_array($value)) { | ||||||||||
| $value = reset($value); | ||||||||||
| } | ||||||||||
| break; | ||||||||||
| default: | ||||||||||
| if (isset($parameter['defaultValue'])) { | ||||||||||
| $this->inputs[$context][$name]['value'] = $parameter['defaultValue']; | ||||||||||
| } | ||||||||||
| $value ??= ($parameter['defaultValue'] ?? null); | ||||||||||
| break; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| unset($parameter); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Copy global parameter values to the guessed context | ||||||||||
| if (array_key_exists('global', $parameters)) { | ||||||||||
| foreach ($parameters['global'] as $name => $parameter) { | ||||||||||
| if (isset($input[$name])) { | ||||||||||
| $value = $input[$name]; | ||||||||||
| } else { | ||||||||||
| if (($parameter['type'] ?? null) === 'checkbox') { | ||||||||||
| $value = false; | ||||||||||
| } elseif (isset($parameter['defaultValue'])) { | ||||||||||
| $value = $parameter['defaultValue']; | ||||||||||
| } else { | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| // Set the value in the GUESSED context. | ||||||||||
| $this->inputs[$queriedContext][$name]['value'] = $value; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Only keep guessed context parameters values | ||||||||||
| if (isset($this->inputs[$queriedContext])) { | ||||||||||
| $this->inputs = [ | ||||||||||
| $queriedContext => $this->inputs[$queriedContext], | ||||||||||
| ]; | ||||||||||
| } else { | ||||||||||
| $this->inputs = []; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| protected function getInput($input) | ||||||||||
|
|
||||||||||
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 moved it out of the loop. We can throw an exception here if wanted.