Skip to content

[4.x] Get reviews aggregations via php#50

Open
indykoning wants to merge 2 commits into
4.xfrom
feature/backend-reviews-aggregation-4.x
Open

[4.x] Get reviews aggregations via php#50
indykoning wants to merge 2 commits into
4.xfrom
feature/backend-reviews-aggregation-4.x

Conversation

@indykoning

Copy link
Copy Markdown
Member

This PR gets the aggregated review counts using the backend. Getting rid of a graphql query requesting a size of 9999
image

Ref: RAP-1572

Comment on lines +62 to +69
$reviewsCountPerStar = [];
for ($i = 1; $i <= $stars; $i++) {
$reviewsCountPerStar[$i] = 0;
foreach ($reviewsGroupedByAveragePercent as $averagePercent => $reviewsCount) {
if (ceil($averagePercent / (100 / $stars)) == $i) {
$reviewsCountPerStar[$i] += $reviewsCount;
}
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Doing it this way ensures we don't drop any ratings in shops that allow multiple ratings to be given per review.
e.g. 5 stars for quality, 4 stars for service would be (100 + 80) / 2 = 90
The previous calculation would leave this out entirely ratings?.filter(e => e.average_rating == {{ $i * 20 }}) while this new calculation rounds it up.

});
});

config('rapidez.models.product')::macro('reviewCountPerStar', function (int $stars = 5) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I find this function quite weird. If we're using stars, that's always a 1-5 scale. But here you can choose whether it's up to 5 or something else. Maybe a more fitting name could be reviewCountBuckets, then it would at least make sense to me.

Comment thread src/Models/Review.php

public function ratingOptionVotes(): HasMany
{
return $this->hasMany(RatingOptionVote::class, 'review_id');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The models config should be used instead of using the model classes directly here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants