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 orderBy to reference associations #1146

Merged
merged 1 commit into from
Oct 22, 2014

Conversation

c960657
Copy link
Contributor

@c960657 c960657 commented Sep 23, 2014

If I specify an order-by clause on a to-many association, e.g. @OrderBy({"foo" = "ASC"}), where foo is a to-one-association on the target entity, the collection is sorted by foo_id (this behaviour is implemented in BasicEntityPersister::getOrderBySQL()).

This works fine. However, Doctrine\ORM\Tools\SchemaValidator complains that foo is not a field.

This change makes the validator accept order-by clauses on associations where the target entity is the owning side, and the association is single-valued (AFAICT this is the same restrictions that BasicEntityPersister uses).

@doctrinebot
Copy link

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-3322

We use Jira to track the state of pull requests and the versions they got
included in.

@Ocramius
Copy link
Member

@c960657 could you provide relevant test cases?

@c960657
Copy link
Contributor Author

c960657 commented Sep 23, 2014

I have now pushed some tests also.

@@ -228,9 +228,19 @@ public function validateClass(ClassMetadataInfo $class)

if (isset($assoc['orderBy']) && $assoc['orderBy'] !== null) {
foreach ($assoc['orderBy'] as $orderField => $orientation) {
if (!$targetMetadata->hasField($orderField)) {
if (!$targetMetadata->hasField($orderField) && !$targetMetadata->hasAssociation($orderField)) {
Copy link
Member

Choose a reason for hiding this comment

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

I would actually change the order of the code to avoid doing the check twice:

 if ($targetMetadata->hasAssociation($orderField)) {
    if (!$targetMetadata->isSingleValuedAssociation($orderField)) {
        // ....
        continue;
    }

    if ($targetMetadata->isAssociationInverseSide($orderField)) {
        // ...
    }

    continue;
}

if (!$targetMetadata->hasField($orderField)) {
    // ....
}

Copy link
Member

Choose a reason for hiding this comment

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

actually, this can even be simplified:

if (!$targetMetadata->hasField($orderField) && !$targetMetadata->hasAssociation($orderField)) {
    // ....

    continue;
}

if ($targetMetadata->isCollectionValuedAssociation($orderField)) {
    // ....
    continue;
}

if ($targetMetadata->isAssociationInverseSide($orderField)) {
    // ...

    continue;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stof Fixed.

@stof
Copy link
Member

stof commented Oct 5, 2014

👍

@guilhermeblanco guilhermeblanco self-assigned this Oct 22, 2014
guilhermeblanco added a commit that referenced this pull request Oct 22, 2014
Allow orderBy to reference associations
@guilhermeblanco guilhermeblanco merged commit 3a0d7d1 into doctrine:master Oct 22, 2014
@webdevilopers
Copy link

Sorry for commenting on a merged PR.

Is there a way to sort by properties of nested entities? E.g. windowTypes.windowType.name?
In my use case the nested entity windowType is indeed sorted but by id by default. Is there away to use name instead? Maybe lookup the __toString() method or some kind of array notation windowType.name or windowType[name]?

class BayWindow
{
    /**
     * @var windowTypes
     *
     * @ORM\OneToMany(targetEntity="BayWindowWindowType", mappedBy="bayWindow")
     * @ORM\OrderBy({"windowType" = "ASC"})
     */
    private $windowTypes;
}

class BayWindowWindowType
{
    /**
     * @var $baywindow;
     *
     * @ORM\ManyToOne(targetEntity="BayWindow", inversedBy="windowTypes")
     * @ORM\JoinColumn(name="bay_window_id", referencedColumnName="id")
     */
    private $bayWindow;

    /**
     * @var $windowType
     *
     * @ORM\ManyToOne(targetEntity="WindowType", inversedBy="bayWindows")
     * @ORM\JoinColumn(name="window_type_id", referencedColumnName="id")
     */
    private $windowType;
}

class WindowType
{
    /**
     * @var string $name
     *
     * @ORM\Column(name="name", type="string", length=100, precision=0, scale=0, nullable=false, unique=false)
     */
    private $name;

    /**
     * @var windowTypes
     *
     * @ORM\OneToMany(targetEntity="BayWindowWindowType", mappedBy="windowType")
     */
    private $bayWindows;

    public function __toString()
    {
        return $this->name;
    }
}

Workaround of course would be using the query builder. But are there any plany for this? Or are there technical downsides?

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.

7 participants