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

Support native enum hydration when using NEW operator #9936

Merged
merged 1 commit into from
Aug 3, 2022

Conversation

romm
Copy link
Contributor

@romm romm commented Jul 27, 2022

Using the NEW operator with the query builder now properly converts
scalar values to native enums inside data transfer objects.

@romm romm force-pushed the fix/enum-new-operator branch from 02e054a to 015cae5 Compare July 28, 2022 06:44
@derrabus derrabus changed the base branch from 2.12.x to 2.13.x July 28, 2022 11:12
Comment on lines 136 to 166
$result = $this->_em->createQueryBuilder()
->from(Card::class, 'c')
->select(new Func('NEW ' . DtoWithEnum::class, ['c.suit']))
->getQuery()
->getResult();

$this->assertInstanceOf(Suit::class, $result[0]->suit);
$this->assertEquals(Suit::Clubs, $result[0]->suit);

$result = $this->_em->createQueryBuilder()
->from(CardWithNullable::class, 'c')
->select(new Func('NEW ' . DtoWithEnum::class, ['c.suit']))
->getQuery()
->getResult();

$this->assertNull($result[0]->suit);
Copy link
Member

Choose a reason for hiding this comment

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

These are actually two separate test cases. Please split them up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. 👍

As it was inspired by a test case above, I modified it as well.

@romm romm force-pushed the fix/enum-new-operator branch from 015cae5 to 3d12fca Compare July 29, 2022 08:19
@SenseException
Copy link
Member

Code looks good so far from my POV. Could you please write the tests using SQL for the NEW operator? Something like 'SELECT NEW ' . DtoWithEnum::class . '(c.suit) FROM ...' to make this more readable?

Using the `NEW` operator with the query builder now properly converts
scalar values to native enums inside data transfer objects.
@romm romm force-pushed the fix/enum-new-operator branch from 3d12fca to 5d434c8 Compare July 31, 2022 20:15
@romm
Copy link
Contributor Author

romm commented Jul 31, 2022

Sure 🙂

@SenseException SenseException requested a review from a team July 31, 2022 20:28
@derrabus derrabus added this to the 2.13.0 milestone Aug 3, 2022
@derrabus derrabus merged commit 33f4db8 into doctrine:2.13.x Aug 3, 2022
@derrabus
Copy link
Member

derrabus commented Aug 3, 2022

Cool. 🙂

@romm romm deleted the fix/enum-new-operator branch August 4, 2022 07:17
derrabus added a commit to derrabus/orm that referenced this pull request Aug 7, 2022
* 2.13.x:
  Address DBAL 3.4 deprecations (doctrine#9969)
  Improve phpdoc for ClassMetadataInfo (doctrine#9965)
  Fix build (doctrine#9964)
  fix: class normalisation test (doctrine#9966)
  Support native enum hydration when using `NEW` operator (doctrine#9936)
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.

3 participants