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

Join type in nested sub-relations #104

Merged
merged 29 commits into from
Oct 2, 2024

Conversation

dmikam
Copy link
Contributor

@dmikam dmikam commented Jul 11, 2022

Possibility to indicate join type for each relation in joinRelationship callbacks:

User::joinRelationship('posts.comments', [
    'posts' => function ($join) {
        $join->where('posts.published', true);
    },
    'comments' => function ($join) {
        $join->where('comments.approved', true);
        $join->left(); // you may also use $join->inner() or directly $join->joinType('leftPowerJoin')
    }
]);

Probable not the most effective way, but the least intrusive.

dmikam and others added 2 commits July 11, 2022 11:44
Documentation for join type in joinRelationship()
…ip callbacks:

 User::joinRelationship('posts.comments', [
    'posts' => function ($join) {
        $join->where('posts.published', true);
    },
    'comments' => function ($join) {
        $join->where('comments.approved', true);
        $join->joinType('leftPowerJoin');
    }
]);

Probable not the most effective way, but the least intrusive.
@luisdalmolin
Copy link
Member

Hi @dmikam, thanks for the contribution. Would you mind adding some tests covering the new features you are adding? I don't feel comfortable merging this code without test coverage.

@luisdalmolin
Copy link
Member

Also, seems like your changes broke the existing tests, so definitely we need to adjust a few things here.

@dmikam
Copy link
Contributor Author

dmikam commented Jul 28, 2022

Tests created. Broken tests fixed too.
By the way found this issue: #105

P.S.: Sorry for that bunch of commits - just was finding out how to work with Github actions and how to make them work in different branch. Also it took me some time to find out that the problem I had was due to that issue Nº105.

@beshoo
Copy link

beshoo commented Aug 31, 2022

Is it safe to use this PL?

@beshoo
Copy link

beshoo commented Nov 15, 2022

@dmikam is this PL fix #105

@dmikam
Copy link
Contributor Author

dmikam commented Nov 16, 2022

@beshoo no, these are separate problems. And about #104 - I use it in current project and it seems pretty safe to use.

@dmikam
Copy link
Contributor Author

dmikam commented Jan 9, 2023

@luisdalmolin May we hope this PR to be included into the next release?

@luisdalmolin
Copy link
Member

@dmikam I'll review this again this week and likely include it. I may merge and apply some changes myself, mostly style, just FYI.

@dmikam
Copy link
Contributor Author

dmikam commented Jul 24, 2023

Just tried the 3.2.0 release, but this functionality isn't there yet... Is there still a hope? Or should I just stick to my modified 2.65 version?

Copy link
Member

@luisdalmolin luisdalmolin left a comment

Choose a reason for hiding this comment

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

This will be in v4 soon

@luisdalmolin luisdalmolin merged commit a5ee29c into kirschbaum-development:master Oct 2, 2024
15 checks passed
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