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

Multiple joins for reaction type counts #97

Merged
merged 11 commits into from
Aug 3, 2019
Merged

Multiple joins for reaction type counts #97

merged 11 commits into from
Aug 3, 2019

Conversation

jfunulab
Copy link
Contributor

@jfunulab jfunulab commented Aug 1, 2019

Hey @acidjazz ,
I fixed a use case of mine which I think might be beneficial to the community so I making this pull request.
The current implementation restricted you to only getting only one reaction type count on a reactant at a time with the joinReactionCounterOfType scope. This pr enables you chain multiple scopes of different reaction types to get all their respective counts at once.
Kindly review and let me know if there are any modifications you will like me to make.

@antonkomarev
Copy link
Member

antonkomarev commented Aug 1, 2019

@JFunu thank you for the contribution!
I was thinking about ability to add multiple counters at once and planned to return to it as a part of PR #93.

What will happen if developer will add 2 joins with different types and without custom alias?

$likeType = ReactionType::fromName('Like');
$dislikeType = ReactionType::fromName('Dislike');

Comment::query()
  ->joinReactionCounterOfType($likeType)
  ->joinReactionCounterOfType($dislikeType);

I think it should work out of the box by default generating alias from the reaction name. And we could add extra $alias nullable parameter which let developer to skip generation of $alias value.

public function scopeJoinReactionCounterOfType(
    Builder $query,
    ReactionTypeContract $reactionType,
    ?string $alias = null
): Builder {

Then this code will join 2 counters with default aliases:

Comment::query()
  ->joinReactionCounterOfType($likeType)
  ->joinReactionCounterOfType($dislikeType);

And this one will join 2 counters with custom aliases:

Comment::query()
  ->joinReactionCounterOfType($likeType, 'likesCounter')
  ->joinReactionCounterOfType($dislikeType, 'dislikesCounter');

I haven't decided how default alias will look like but it should depend of reaction name for sure, because they are unique.

@antonkomarev antonkomarev added this to the v8.0.0 milestone Aug 1, 2019
src/Reactable/Models/Traits/Reactable.php Outdated Show resolved Hide resolved
src/Reactable/Models/Traits/Reactable.php Outdated Show resolved Hide resolved
src/Reactable/Models/Traits/Reactable.php Outdated Show resolved Hide resolved
@jfunulab
Copy link
Contributor Author

jfunulab commented Aug 2, 2019

@antonkomarev . Thanks for the feedback. Have implemented the changes.

@antonkomarev
Copy link
Member

Looks good! The only one things left to do - add test for join with custom $alias argument and I'm ready to merge it.

@antonkomarev antonkomarev merged commit 9b30f86 into cybercog:master Aug 3, 2019
@antonkomarev
Copy link
Member

antonkomarev commented Aug 3, 2019

Nevermind, I've merged it as is. I will add tests myself in next PR #102.

@JFunu you are in Contributors list now! 🚀

@antonkomarev antonkomarev changed the title multiple joins for reaction type counts Multiple joins for reaction type counts Aug 3, 2019
@jfunulab
Copy link
Contributor Author

jfunulab commented Aug 3, 2019

Thanks @antonkomarev . My first contribution to open source.

@antonkomarev
Copy link
Member

Welcome Aboard! 🥇

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.

None yet

2 participants