composer / packagist

Package Repository Website - try https://packagist.com if you need your own -
https://packagist.org/
MIT License
1.76k stars 477 forks source link

list.json cannot take multiple filters #1487

Open tacman opened 4 days ago

tacman commented 4 days ago

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 commented 3 days ago

this is implemented in https://github.com/composer/packagist/blob/aeb9a88cffaf6cedeb6620c5dcdd307e9cb8c982/src/Controller/PackageController.php#L100

tacman commented 3 days ago

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

tacman commented 3 days ago

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 commented 3 days ago

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

tacman commented 3 days ago

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