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

Add Criteria::fromFindCriteria #89

Open
wants to merge 1 commit into
base: 2.0.x
Choose a base branch
from

Conversation

soullivaneuh
Copy link
Contributor

Subject

The goals is to be able to quick reproduce a criteria looking like parameters given to ORM find methods.

Concrete case

On a REST controller action, I would like to setup basic filter and apply them with the findBy method of the ORM.

I have a Server entity containing many User entities.

When the user calling the api method is not an admin, I have to filter the server list to get only related ones.

But the findBy does not allow us to do a search on a ManyToMany relationship.

I would like to keep same method on criteria to apply it on a query builder:

public function cgetAction(ParamFetcherInterface $paramFetcher)
{
    $view = $this->view();

    $filters = [
        'name' => $paramFetcher->get('name'),
    ];
    $filters = array_filter($filters, function ($value) {
        return null !== $value;
    });

    if ($this->get('admin.server')->isGranted('LIST')) {
        $servers = $this->getDoctrine()->getRepository('AppBundle:Server')->findBy($filters);
        $view->getSerializationContext()->setGroups(['list', 'list_admin']);
    } else {
        $servers = $this->getDoctrine()->getRepository('AppBundle:Server')->getByOwnerQB($this->getUser())
            ->addCriteria(Criteria::fromFindCriteria($filters))
            ->getQuery()->getResult();
        $view->getSerializationContext()->setGroups('list');
    }

    $view->setData($servers);

    return $view;
}

Having this method would simplify the logic.

@@ -76,6 +76,24 @@ public static function create()
}

/**
* Creates an instance with same behaviour as criteria parameters passed on ORM find methods.
*
* @param array $findCriteria
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"array" => "Criteria[]" ? Not sure what is used in this project…

Copy link
Contributor Author

@soullivaneuh soullivaneuh Jun 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this is the same as ObjectRepository::findBy method first parameter.

Ref: https://github.com/doctrine/common/blob/faf7d81273eeffb84d524006daf85d37a14ac42a/lib/Doctrine/Common/Persistence/ObjectRepository.php#L55

I'll add a comment.

* Creates an instance with same behaviour as criteria parameters passed on ORM find methods.
*
* @param mixed[] $findCriteria Array of different keys and values. Gets the same format as argument expected on
* ObjectRepository::findBy and ObjectRepository::findOneBy methods.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use fully qualified class names, as they are in a different package

@soullivaneuh
Copy link
Contributor Author

@stof fixed.

@soullivaneuh
Copy link
Contributor Author

Any news on this PR?

@mikeSimonson
Copy link
Contributor

@soullivaneuh As far as I can see in the entityPersister, if you want to make the 2 compatible you have to take into account 3 more cases and their tests.

Thanks

@soullivaneuh
Copy link
Contributor Author

@mikeSimonson Okay I'll see to update the tests first.

Copy link
Member

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also address this (after a rebase to sync with master).

@@ -16,6 +16,18 @@ public function testCreate() : void
$this->assertInstanceOf(Criteria::class, $criteria);
}

public function testFromFindCriteria()
{
$criteria = Criteria::fromFindCriteria(array('name' => 'test', 'foo' => 42));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use short array syntax


/** @var CompositeExpression $where */
$where = $criteria->getWhereExpression();
$this->assertInstanceOf('Doctrine\Common\Collections\Expr\CompositeExpression', $where);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ::class syntax

The goals is to be able to quick reproduce a criteria looking like parameters given to ORM find methods.
@soullivaneuh
Copy link
Contributor Author

soullivaneuh commented Jun 27, 2017

@mikeSimonson I updated the code to manage the first case.

Stop me if I'm wrong, but I don't thing I have anything to do for the null cases.

This will be simply managed by this:

static::expr()->eq($key, $value);

Where $value will be null.

@Ocramius Ocramius requested a review from mikeSimonson June 28, 2017 04:29
Base automatically changed from master to 2.0.x January 19, 2021 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants