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

Orm querybuilder set parameters to collection #326

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marcelthole
Copy link
Contributor

hey,

i have a new feature proposal for changing the method signature for Doctrine\ORM\QueryBuilder::setParameters from the array type from Doctrine ORM 2.x to the ArrayCollection with Parameter type of ORM 3.x

see doctrine/orm#9490
see https://github.com/doctrine/orm/blob/3.0.x/UPGRADE.md#query-querybuilder-and-nativequery-parameters-bc-break

@@ -163,6 +164,6 @@ private function isNullableJoinColumn(Property $property): bool
self::JOIN_COLUMN,
'nullable'
);
return $joinExpr instanceof Expr\ConstFetch && ! in_array('false', $joinExpr->name->getParts(), true);
return $joinExpr instanceof ConstFetch && ! in_array('false', $joinExpr->name->getParts(), true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh this change was not planed. I guess the composer fix-cs changed this

@JoolsMcFly
Copy link

Hi.

I was about to get cracking on this and saw your PR @marcelthole 👍

It would be nice if you could transform the following:

$params = ['name' => 'John'];
if ($someCondition) {
    $qb->andWhere('age > :age');
    $params['age'] = 18;
}
$qb->setParameters($params);

into

$params = new ArrayCollection([new Parameter('name', 'John')]);
if ($someCondition) {
    $qb->andWhere('age > :age');
    $params->add(new Parameter('age', 18));
}
$qb->setParameters($params);

@marcelthole
Copy link
Contributor Author

@JoolsMcFly i like the idea and created a second PR for that here: #346
if this one gets merged we could check the addition :)

@@ -0,0 +1,27 @@
<?php

Copy link
Member

Choose a reason for hiding this comment

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

please add namespace like other fixture, non-namespaced fixture usually only on specific usecase

*/
public function refactor(Node $node): Node|null
{
assert($node instanceof MethodCall);
Copy link
Member

Choose a reason for hiding this comment

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

assert is not needed

Suggested change
assert($node instanceof MethodCall);

…ection for the setParameters ORM QueryBuilder method
@marcelthole marcelthole force-pushed the orm-querybuilder-setParameters-to-collection branch from 23348db to b2f4a01 Compare September 20, 2024 09:07
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 this pull request may close these issues.

3 participants