Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Added leftjoin ORM Query Type #42

Merged
merged 4 commits into from Jan 16, 2018

Conversation

TomHAnderson
Copy link

I found myself needing to query the non-foreign-key containing side of a relationship for null on the other and innerjoin does not do that, so here is leftjoin.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 86.27% when pulling 18b3540 on TomHAnderson:feature/leftjoin into c5c483e on zfcampus:master.

Copy link
Member

@michalbundyra michalbundyra left a comment

Choose a reason for hiding this comment

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

@TomHAnderson thanks for this feature. Code looks good for me, just two small comments. I haven't tested it yet, but I believe you checked it and it works for you and you 😄 Thanks 👍

README.md Outdated
@@ -303,6 +303,16 @@ To enable inner join add this to your configuration.
```


There is also an ORM Query Type for LeftJoin. This join type is commonly used to fetch an empty right side of a relationship.

Left Join is not included by default in the `zf-doctrine-querybuilder.global.php.dist`.
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason why it is not included in the dist config file?

Copy link
Author

Choose a reason for hiding this comment

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

For the same reason innerJoin is not included in the .dist. The join filters allow deeper querying of data. Not including them is erring on the side of caution. Examples are all theoretical. Including the ability to join tables IMHO should be left to the implementing developer.

For instance given a resource which can return users you could inner join the user table to the user acl table and fetch all users who are just administrators.

Copy link
Member

Choose a reason for hiding this comment

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

Fine. Don't you think then we need add here how to enable it? I know it will be very similar to innerjoin, but maybe better to have it here, and if someone want enable it can just copy-paste this part from the docs. Or maybe even better, add it to .dist config but commented out?

Copy link
Author

Choose a reason for hiding this comment

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

I've removed the instructions to add the filter yourself and I've added both join filters to the .dist config file. TomHAnderson@f7d9119

<?php
/**
* @license http://opensource.org/licenses/BSD-3-Clause BSD-3-Clause
* @copyright Copyright (c) 2014 Zend Technologies USA Inc. (http://www.zend.com)
Copy link
Member

Choose a reason for hiding this comment

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

Please update year to 2017 here.

Copy link
Author

Choose a reason for hiding this comment

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

Odd behavior of github right now. I pushed this change to the PR branch and it didn't add the commit here. https://github.com/TomHAnderson/zf-doctrine-querybuilder/commits/feature/leftjoin

@michalbundyra
Copy link
Member

@TomHAnderson No idea why github is not updating your commits here... So weird!

@TomHAnderson
Copy link
Author

Github status says there's a backlog of queue events. I'll check back later today.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 86.27% when pulling f7d9119 on TomHAnderson:feature/leftjoin into c5c483e on zfcampus:master.

@TomHAnderson
Copy link
Author

This PR now includes all expected commits.

@michalbundyra
Copy link
Member

@TomHAnderson Thanks !

@michalbundyra michalbundyra added this to the 1.6.0 milestone Sep 21, 2017
@michalbundyra michalbundyra self-assigned this Jan 16, 2018
@michalbundyra michalbundyra changed the base branch from master to develop January 16, 2018 17:51
@michalbundyra michalbundyra merged commit f7d9119 into zfcampus:develop Jan 16, 2018
michalbundyra added a commit that referenced this pull request Jan 16, 2018
michalbundyra added a commit that referenced this pull request Jan 16, 2018
@michalbundyra
Copy link
Member

@TomHAnderson thanks !

michalbundyra added a commit that referenced this pull request Jan 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants