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

WhereIdIn filter doesn't work #68

Closed
mspiderv opened this issue Apr 9, 2021 · 5 comments
Closed

WhereIdIn filter doesn't work #68

mspiderv opened this issue Apr 9, 2021 · 5 comments
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@mspiderv
Copy link

mspiderv commented Apr 9, 2021

Version: v1.0.0-beta.1

Im getting this error "Expecting filter value to be an array, or a string when a string delimiter is set.".

URL Im hitting: http://localhost:8000/api/jsonapi/product-categories?filter[id]=3

My filters method in Schema class:

public function filters(): array
    {
        return [
            WhereIdIn::make($this),
        ];
    }

I think the problem is here: /vendor/laravel-json-api/eloquent/src/Filters/Concerns/HasDelimiter.php on line 53:

protected function toArray($value): array
    {
        if ($this->delimiter && \is_string($value)) {
            return \explode($this->delimiter, $value);
        }

        if (\is_array($value)) {
            return $value;
        }

        throw new \LogicException(
            'Expecting filter value to be an array, or a string when a string delimiter is set.'
        );
    }

This method throws that error, because the $value is simply a string.

I think this is a bug.

@lindyhopchris
Copy link
Contributor

lindyhopchris commented Apr 9, 2021

You should validate the id filter query parameter to be an array or a string, depending on which one you're using.

@lindyhopchris
Copy link
Contributor

This:

?filter[id]=3

Is a string, so you should validate the id filter to be a string. If you were searching for multiple records, you would use:

?filter[id]=3,5

When it's a string (and assuming your delimiter is ,).

If you intend to use an array, the format of the id filter should be:

?filter[id][]=3

And for multiple:

?filter[id][]=3&filter[id][]=5

@mspiderv
Copy link
Author

mspiderv commented Apr 9, 2021

Thanks for info. I thought it might be some problem with validation, but I wasn't able to find any information in docs (here: https://laraveljsonapi.io/docs/1.0/schemas/filters.html). There is not a single word about validationg query params on this particular docs subpage.

Also here https://laraveljsonapi.io/docs/1.0/schemas/filters.html#whereidin is wrong example.

This is a copy-paste from the docs:

GET /api/v1/posts?filter[id]=3&filter[id]=9 HTTP/1.1
Accept: application/vnd.api+json

As you can see, you are not passing the ID as an array.

@lindyhopchris
Copy link
Contributor

Ah right yeah so that's a typo in the docs.

There's a whole chapter on query parameter validation here:
https://laraveljsonapi.io/docs/1.0/requests/query-parameters.html

But you're right that maybe the two chapters aren't cross-referenced enough.

I'll reopen this issue to remind me to fix the docs.

@lindyhopchris lindyhopchris reopened this Apr 9, 2021
@lindyhopchris lindyhopchris added the documentation Improvements or additions to documentation label Apr 9, 2021
@lindyhopchris lindyhopchris added this to the v1.0.0-beta.2 milestone Apr 9, 2021
@lindyhopchris
Copy link
Contributor

I've:

  1. Fixed the typos in the docs
  2. Added a section cross-referencing the filter chapter to the query parameters validation chapter
  3. Added tips to the WhereIdIn and WhereIdNotIn filter sections to sate that you need to validate values as either the array or the string, depending on which you're supporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants