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

Also include empty string and comment types for backward compat. #28624

Merged
merged 2 commits into from Dec 21, 2020

Conversation

vedanshujain
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

In #26928, we added WHERE clause to only count the reviews which have review comment type. However, we also display comments with empty string or comment in type column. This means that those reviews are displayed but are not counted.

This PR fixes the issue by also counting reviews with empty string or comment in the comment type column (since we also display them).

This fixes the root cause for #27688, but to close the issue we would need a migration that will correct the review count for existing products.

How to test the changes in this Pull Request:

  1. Add a new review comment for a product via admin.
  2. With this PR, you will see the count updated in frontend view, without this PR the new comment will not be counted.

Note that the counter for review comment is cached in meta value for product, so already existing reviews will still not be counted unless the postmeta with meta key _wc_rating_count for the product is deleted or a new review is added.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Fix - Also count comment types with '' and 'comment' in the review count query.

@vedanshujain vedanshujain requested review from a team and rodrigoprimo and removed request for a team December 18, 2020 13:20
@rodrigoprimo rodrigoprimo removed their request for review December 19, 2020 20:21
@rodrigoprimo
Copy link
Contributor

@vedanshujain, I'm sorry, but I didn't have time to review this PR before my end of the year vacation. That is why I unassigned myself from it.

@vedanshujain vedanshujain added the release: highlight Issues that have a high user impact and need to be discussed/paid attention to. label Dec 21, 2020
Copy link
Contributor

@claudiosanches claudiosanches left a comment

Choose a reason for hiding this comment

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

Works fine!

@claudiosanches claudiosanches merged commit 8cc9539 into master Dec 21, 2020
@claudiosanches claudiosanches deleted the fix/27688 branch December 21, 2020 15:11
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Dec 21, 2020
@claudiosanches claudiosanches added this to the 4.9.0 milestone Dec 21, 2020
@vedanshujain vedanshujain removed the release: add changelog Mark all PRs that have not had their changelog entries added. [auto] label Dec 21, 2020
@juliaamosova juliaamosova added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: highlight Issues that have a high user impact and need to be discussed/paid attention to.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants