Support for Join conditions

[ju1ius]

This:

Spec::join('users', 'user', null, Spec::andx(
    Spec::eq('active', true),
    Spec::eq('status', 'somestatus')
))

Will be equivalent to:

$qb->join('foo.users', 'user', 'WITH', $qb->expr()->andX(
    $qb->expr()->eq('user.active',  true),
    $qb->expr()->eq('user.status', 'somestatus')
));

[Nyholm]

Shouldn't it be the other way around? The signature of the QueryBuilder::join is:

public function join($join, $alias, $conditionType = null, $condition = null, $indexBy = null);

I think we should have it in the same order.

[ju1ius]

I chose this order because 90% of the time I don't need to change the default ON clause.
It makes the api less verbose, but on the other hand it's not as expressive and diverges from Doctrine, so it might be confusing...

Maybe we should keep it the doctrine way, and add a new method to the builder, like

Spec::with($field, $newAlias, $joinType, $joinCondition);

While we're at it we could also solve the problem that it's currently impossible to do fetch joins with the spec builder:

namespace Happyr\DoctrineSpecification\Query;

class With extends AbstractJoin
{
    protected $joinType;

    public function __construct ($field, $newAlias, $joinType='join',  $condition=null) 
    {
        $parentAlias = substr($field, 0, strpos($field, '.'));
        $this->joinType = $joinType;
        parent::__construct($field, $newAlias, $parentAlias ?: null, 'WITH', $condition);
    }

    public function modify(QueryBuilder $qb, $dqlAlias)
    {
        $dqlAlias = $this->dqlAlias ?: $dqlAlias;
        $join = $this->joinType;
        $condition = $this->getJoinCondition($this->condition, $qb);
        $qb
            ->addSelect($this->newAlias) // FETCH THAT JOIN !!!
            ->$join("{$dqlAlias}.{$this->field}", $this->newAlias, 'WITH', $condition)
        ;
    }
}

[Nyholm]

I agree what the order doctrine uses is verbose. But it makes sense because it is easier to read and understand.

I don't really like the idea with new With, those kind of shortcuts should be in the factory. I suggest changing the constructor to be the same as Doctrine and then add a factory method which covers the 90% of the use cases:

public static function joinWith($field, $newAlias, $condition, $dqlAlias = null, $conditionType = null)
{
    return new Join($field, $newAlias, $dqlAlias, $conditionType, $condition);
}

What do you think about that?

[peter-gribanov]

There is no need for the condition type.
For using conditions in join you can use only WITH. If you try use ON you get the error:

An exception has been thrown during the rendering of a template ("[Syntax Error] line 0, col 137: Error: Expected Doctrine\ORM\Query\Lexer::T_WITH, got 'ON'").

See this doctrine/orm#7193 issue for more detales.

[Nyholm]

Thank you for this PR. This sure is a missing features. I just hade some minor comments.

[Nyholm]

We should define our own ON and WITH constants. We don't want to be dependent on Doctrine.

[Nyholm]

Hey. I would really like to merge this? Any estimates when an update to this PR can be made?

[peter-gribanov]

I consider this change to be unnecessary.
This is unnecessary over-complication.

This method mixes the level of business logic and persistence level.

The same result can be achieved easier:

Spec::andX(
    Spec::join('users', 'u'),
    Spec::eq('active', true, 'u'),
    Spec::eq('status', 'somestatus', 'u')
)

I prefer not to use the library directly, but i create specifications, the name of which describes the business rule:

Spec::andX(
    new JoinCommentUser('u'),
    new PublishedUser('u'),
    new UserWithStatus('somestatus', 'u')
)

[peter-gribanov]

The new context system added in version 2 makes using relationships even easier.

Spec::andX(
    Spec::eq('active', true, 'users'),
    Spec::eq('status', 'somestatus', 'users')
)