Skip to content
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

5.x: Search exception #338

Closed
dereuromark opened this issue Dec 4, 2023 · 5 comments · Fixed by #342
Closed

5.x: Search exception #338

dereuromark opened this issue Dec 4, 2023 · 5 comments · Fixed by #342
Labels

Comments

@dereuromark
Copy link
Member

I have params of

'city_id' => (int) 76,

Which then triggers exception

Search\Model\Filter\Base::passedValue(): Return value must be of type array|string|null, int returned

    /**
     * @return array|string|null
     */
    protected function passedValue(): string|array|null

I wonder if this should be also allowing other scalar values actually?
As it seems hard to control the exact types going into the search here IMO

@dereuromark dereuromark added the bug label Dec 4, 2023
@dereuromark
Copy link
Member Author

I worked around it for now by casting

$search['city_id'] = (string)$city->id;

before passing those further on.

@ADmad
Copy link
Member

ADmad commented Dec 4, 2023

How/why is city_id an int? Passed values normally come request query string which are always strings.

@dereuromark
Copy link
Member Author

The data doesnt completely come from the request, thats why
It seems strange to have to take a primary key int, make it string only to become int afterwards again, though.

@ADmad
Copy link
Member

ADmad commented Dec 4, 2023

If we are going to widen the type then let's just make it mixed instead of limiting to scalars and array. This value is eventually going to be used in a query which does accept various types of objects depending on the field type. Also Base::value() which calls passedValue() already has mixed return type.

@dereuromark
Copy link
Member Author

Turns out, the wrapping method already expects mixed.

/**
 * Get the value of the "name" from HTTP GET arguments.
 *
 * @return mixed
 */
public function value(): mixed
{
    $value = $this->_config['defaultValue'];

    $passedValue = $this->passedValue();
    if ($passedValue === null) {
        return $value;
    }

    return $passedValue;
}

So, yeah, mixed is the correct way to go

@dereuromark dereuromark linked a pull request Feb 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants