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

list.json cannot take multiple filters #1487

Closed
tacman opened this issue Nov 18, 2024 · 5 comments · Fixed by #1488
Closed

list.json cannot take multiple filters #1487

tacman opened this issue Nov 18, 2024 · 5 comments · Fixed by #1488

Comments

@tacman
Copy link
Contributor

tacman commented Nov 18, 2024

listing by vendor or type works as expected:

But combining the filters doesn't:

I might be able to submit a PR if someone can point me to the file where this happens.

@stof
Copy link
Contributor

stof commented Nov 19, 2024

this is implemented in

public function listAction(Request $req): JsonResponse

@tacman
Copy link
Contributor Author

tacman commented Nov 19, 2024

My thought is to refactor that to use a QueryBuilder. Or do you have another suggestion?

@tacman
Copy link
Contributor Author

tacman commented Nov 19, 2024

Since this is running on Symfony 7, is there any reason not to take advantage of the cool features later Symfony versions offer, e.g.

    public function listAction(Request $req,
        #[MapQueryParameter] ?string $type=null,
        #[MapQueryParameter] ?string $vendor=null
    ): JsonResponse

@stof
Copy link
Contributor

stof commented Nov 19, 2024

@tacman I don't see how this is relevant to this issue about supporting to combine filters instead of applying them separately.

@tacman
Copy link
Contributor Author

tacman commented Nov 19, 2024

What do you think about combining this into a single method getPackageNamesByVendorAndType()?

        if ($req->query->get('type')) {
            $names = $repo->getPackageNamesByType($req->query->get('type'));
        } elseif ($req->query->get('vendor')) {
            $names = $repo->getPackageNamesByVendor($req->query->get('vendor'));

to

        if ($type || $vendor ) {
            $names = $repo->getPackageNamesByVendorAndType($vendor, $type);
       }

The above looks cleaner to me, thus the MapQueryParameter

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

Successfully merging a pull request may close this issue.

2 participants