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

Allow using objects as discriminators #6141

Merged
merged 1 commit into from
Nov 23, 2016

Conversation

lcobucci
Copy link
Member

@lcobucci lcobucci commented Nov 20, 2016

@Ocramius this is the small change to make it work on master.

The test passes without any change on 2.5-x so it looks like we have a small break on master which is fixed with this changeset.

{
parent::setUp();

Type::addType(GH6141PeopleType::NAME, __NAMESPACE__ . '\\GH6141PeopleType');
Copy link
Member

Choose a reason for hiding this comment

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

Can you please use ::class here?

Type::addType(GH6141PeopleType::NAME, __NAMESPACE__ . '\\GH6141PeopleType');

$this->_schemaTool->createSchema(array(
$this->_em->getClassMetadata(__NAMESPACE__ . '\\GH6141Person'),
Copy link
Member

Choose a reason for hiding this comment

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

same


$this->_schemaTool->createSchema(array(
$this->_em->getClassMetadata(__NAMESPACE__ . '\\GH6141Person'),
$this->_em->getClassMetadata(__NAMESPACE__ . '\\GH6141Boss'),
Copy link
Member

Choose a reason for hiding this comment

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

same

$this->_schemaTool->createSchema(array(
$this->_em->getClassMetadata(__NAMESPACE__ . '\\GH6141Person'),
$this->_em->getClassMetadata(__NAMESPACE__ . '\\GH6141Boss'),
$this->_em->getClassMetadata(__NAMESPACE__ . '\\GH6141Employee'),
Copy link
Member

Choose a reason for hiding this comment

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

same

));
}

public function testEnumDiscriminatorsShouldBeConvertedToString()
Copy link
Member

Choose a reason for hiding this comment

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

One last thing to add to this test: asserting that the discriminator is an object.

Copy link
Member Author

@lcobucci lcobucci Nov 23, 2016

Choose a reason for hiding this comment

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

How would I do that? The retrieved object doesn't have the discriminator and the entity metadata contains it as a string

Copy link
Member

Choose a reason for hiding this comment

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

Probably via metadata and $metadata->getDiscriminatorValue(), or by hydrating a row as array.

Copy link
Member Author

Choose a reason for hiding this comment

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

done 😉

$this->_em->flush();
$this->_em->clear();

$query = $this->_em->createQueryBuilder()
Copy link
Member

Choose a reason for hiding this comment

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

findOneBy() instead - simpler here, as we don't go through the entire DQL scenarios

Copy link
Member Author

Choose a reason for hiding this comment

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

With that findOneBy() would use the SimpleObjectHydrator instead of the ObjectHydrator.

Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment about that then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do

return new self($value);
}

public static function isValid($valid)
Copy link
Member

Choose a reason for hiding this comment

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

Remove or make private, since it's not used in the examples


private $value;

public static function get($value)
Copy link
Member

Choose a reason for hiding this comment

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

Docblocks for types here (methods/properties)

* @InheritanceType("JOINED")
* @DiscriminatorColumn(name="discr", type="gh6141people")
* @DiscriminatorMap({
* "boss" = "Doctrine\Tests\ORM\Functional\Ticket\GH6141Boss",
Copy link
Member

Choose a reason for hiding this comment

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

Can use GH6141Boss::class here, instead of a string

* @DiscriminatorColumn(name="discr", type="gh6141people")
* @DiscriminatorMap({
* "boss" = "Doctrine\Tests\ORM\Functional\Ticket\GH6141Boss",
* "employee" = "Doctrine\Tests\ORM\Functional\Ticket\GH6141Employee"
Copy link
Member

Choose a reason for hiding this comment

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

Can use GH6141Employee::class here, instead of a string

@Ocramius Ocramius added this to the 2.6.0 milestone Nov 23, 2016
@Ocramius Ocramius self-assigned this Nov 23, 2016

Type::addType(GH6141PeopleType::NAME, GH6141PeopleType::class);

$this->_schemaTool->createSchema(array(
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll just convert this to short-array notation

@Ocramius
Copy link
Member

Aaaaaaand merged! Thanks @lcobucci! 👍

@Ocramius Ocramius merged commit 7789df3 into doctrine:master Nov 23, 2016
lcobucci added a commit to lcobucci/doctrine2 that referenced this pull request Nov 23, 2016
@lcobucci lcobucci deleted the object-as-discriminators branch November 23, 2016 18:24
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.

None yet

2 participants