Skip to content

Add configuration to define two identifier but with OR condition #2324

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

Closed
soullivaneuh opened this issue Nov 13, 2018 · 8 comments
Closed

Add configuration to define two identifier but with OR condition #2324

soullivaneuh opened this issue Nov 13, 2018 · 8 comments

Comments

@soullivaneuh
Copy link
Contributor

For example, add the possibility to add identifiers to $id and $name properties, but not consider them as composite identifier.

In that case, APIP will look at the $id first, then $name.

Related to #2323 debate.

@soyuka
Copy link
Member

soyuka commented Nov 13, 2018

#2126 maybe?

@soullivaneuh
Copy link
Contributor Author

@soyuka Not sure. Here is a concrete case:

class Server
{
    /**
     * @var int
     *
     * @ORM\Column(type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     * @Groups("gets")
     */
    private $id;

    /**
     * @var string
     *
     * @ORM\Column(type="string", length=64, unique=true)
     * @Groups({"gets", "post:admin"})
     * @ApiFilter(SearchFilter::class, strategy="partial")
     */
    private $name = '';
}

I would like APIP to search on $id first, then on $name but not both combined.

/api/servers/1 should work.
/api/servers/ns42442 should work too.

If your PR is resolving this need, so yeah it can close this issue. 👍

@soyuka
Copy link
Member

soyuka commented Nov 13, 2018

I figured out better and I have to be 👎 because this is not good practice, an identifier should be unique. In this case I'd only use the $name.

@soullivaneuh
Copy link
Contributor Author

soullivaneuh commented Nov 13, 2018

because this is not good practice

Maybe. But customer needs do not always fit the best practices. Both are needed here.

I can understand if you won't make it native, I will be okay with my custom data provider then. 👍

@norkunas
Copy link
Contributor

This is a common case when you generate slugs for resources..

But I've solved this with another solution without need to create data provider:

<?php

declare(strict_types=1);

namespace App\EventSubscriber\Company;

use ApiPlatform\Core\EventListener\EventPriorities;
use App\Entity\Company;
use App\Repository\CompanyRepository;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class ResolveIdFromSlug implements EventSubscriberInterface
{
    private $repository;

    public function __construct(CompanyRepository $companyRepository)
    {
        $this->companyRepository = $companyRepository;
    }

    public function resolve(GetResponseEvent $event): void
    {
        $request = $event->getRequest();

        if (!\in_array($request->attributes->get('_route'), ['api_companies_get_item', 'api_companies_put_item', 'api_companies_delete_item'], true)) {
            return;
        }

        $identifier = $request->attributes->get('id');

        if (\ctype_digit($identifier)) {
            return;
        }

        $company = $this->companyRepository->findOneBySlug($identifier);

        if (null !== $company) {
            $request->attributes->set('id', $company->getId());
        }
    }

    public static function getSubscribedEvents(): array
    {
        return [
            KernelEvents::REQUEST => ['resolve', EventPriorities::PRE_READ],
        ];
    }
}

@soyuka
Copy link
Member

soyuka commented Nov 14, 2018

You could also avoid the slug query here by using the identifier denormalizer + #2323 (comment)

@norkunas
Copy link
Contributor

You could also avoid the slug query here by using the identifier denormalizer + #2323 (comment)

Thanks, will check it.

@teohhanhui
Copy link
Contributor

Also -1 here for the same reasons.

@soyuka soyuka closed this as completed Dec 14, 2020
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

No branches or pull requests

4 participants