Skip to content

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.

Sign up for GitHub

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

Open
papaj-na-wrotkach wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix: simplify input handling and make it consistent #4894

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9782cdc
fix: simplify input handling and make it consistent
papaj-na-wrotkach Feb 10, 2026
dbea51b
fixup! fix: simplify input handling and make it consistent
papaj-na-wrotkach Feb 10, 2026
f782e03
Merge branch 'master' into fix-input-handling
papaj-na-wrotkach Feb 10, 2026
f8f0cdf
Merge branch 'master' into fix-input-handling
papaj-na-wrotkach Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 23 additions & 58 deletions lib/BridgeAbstract.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,83 +183,48 @@ public function setInput(array $input)

private function setInputWithContext(array $input, $queriedContext)
{
// Import and assign all inputs to their context
foreach ($input as $name => $value) {
foreach ($this->getParameters() as $context => $set) {
if (array_key_exists($name, $this->getParameters()[$context])) {
$this->inputs[$context][$name]['value'] = $value;
}
}
$parameters = $this->getParameters();
if (!array_key_exists($queriedContext, $parameters)) {
// unknown context provided by client, throw exception here? or continue?

@papaj-na-wrotkach papaj-na-wrotkach Feb 10, 2026

Copy link
Copy Markdown
Contributor Author

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.

Comment thread
papaj-na-wrotkach marked this conversation as resolved.
}

// Apply default values to missing data
$contextNames = [$queriedContext];
if (array_key_exists('global', $this->getParameters())) {
if (array_key_exists('global', $parameters)) {
$contextNames[] = 'global';
}

foreach ($contextNames as $context) {
if (!isset($this->getParameters()[$context])) {
// unknown context provided by client, throw exception here? or continue?
}

foreach ($this->getParameters()[$context] as $name => $properties) {
if (isset($this->inputs[$context][$name]['value'])) {
foreach ($parameters[$context] as $name => $properties) {
// Skip this property if we already set the value when iterating through another context.
if (isset($this->inputs[$queriedContext][$name]['value'])) {
continue;
}

$type = $properties['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 ($properties['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

@papaj-na-wrotkach papaj-na-wrotkach Feb 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make checkbox values boolean only, not mixed (value or false), by ignoring the value. The value should not be used by anything - it's a checkbox after all and it can only be set or unset.

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
// We don't use default value. Use the value if value is set, `false` if not.
$value ??= false;
// We don't use default value. Use `true` if value is set, `false` if not.
$value ??= $value !== null;

break;

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refactor changes core input/default handling logic (checkbox/list defaults and global-vs-context fallback), but there are no unit tests covering BridgeAbstract::setInput()/setInputWithContext(). Consider adding focused tests (e.g., checkbox missing => false, list missing => default/first value, global fallback, and invalid/unknown context hint behavior) to prevent regressions.

Copilot uses AI. Check for mistakes.

@papaj-na-wrotkach papaj-na-wrotkach Feb 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I add tests?

case 'list':
if (!isset($properties['defaultValue'])) {
$firstItem = reset($properties['values']);
if (is_array($firstItem)) {
$firstItem = reset($firstItem);
}
$this->inputs[$context][$name]['value'] = $firstItem;
} else {
$this->inputs[$context][$name]['value'] = $properties['defaultValue'];
$value ??= ($properties['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($properties['values']);
if (is_array($value)) {
$value = reset($value);
}
break;
default:
if (isset($properties['defaultValue'])) {
$this->inputs[$context][$name]['value'] = $properties['defaultValue'];
}
$value ??= ($properties['defaultValue'] ?? null);
break;
}
}
}

// Copy global parameter values to the guessed context
if (array_key_exists('global', $this->getParameters())) {
foreach ($this->getParameters()['global'] as $name => $properties) {
if (isset($input[$name])) {
$value = $input[$name];
} else {
if (($properties['type'] ?? null) === 'checkbox') {
$value = false;
} elseif (isset($properties['defaultValue'])) {
$value = $properties['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)
Expand Down