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

Feature: Add support for many_many and through relationships #17

Merged
merged 4 commits into from
Oct 18, 2021

Conversation

chrispenny
Copy link
Member

@chrispenny chrispenny commented Oct 7, 2021

Fixes #12
Closes #12

@chrispenny chrispenny force-pushed the feature/many-many-support branch 2 times, most recently from f829c5f to 6181816 Compare October 7, 2021 00:44
@chrispenny chrispenny force-pushed the feature/many-many-support branch from 6181816 to dabb36e Compare October 7, 2021 01:06
@chrispenny chrispenny marked this pull request as ready for review October 7, 2021 01:07
Copy link
Contributor

@adrhumphreys adrhumphreys left a comment

Choose a reason for hiding this comment

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

I've read through the code and understand most of it, I think we are at a very complex point with the many many which is both nice from a dev using the library perspective (as we've solved a complex problem) but makes life trickier from a maintenance perspective.

I'm not sure how we make it simpler though, maybe we need to think about a high level doc explaining the inner workings or something to that effect to make life easier for other people looking into it

if (array_key_exists($relation, $manyMany) && is_array($touchClassName)) {
$edge = $this->getTouchesManyManyThroughEdge($node, $relation, $touchClassName);

// There is a chance that there was no valid Edge (for a reason that we do understand, and did not
Copy link
Contributor

Choose a reason for hiding this comment

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

If we understand the reason we should state it here

@adrhumphreys adrhumphreys merged commit ae18ecd into main Oct 18, 2021
@adrhumphreys adrhumphreys deleted the feature/many-many-support branch October 18, 2021 00:19
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.

Enhancement: Add support for many_many
2 participants