Skip to content

Add migration to fix incorrect product review count. #28814

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.

Sign up for GitHub

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

Jump to bottom
Merged
roykho merged 4 commits into from
Jan 18, 2021
Merged

Add migration to fix incorrect product review count. #28814

roykho merged 4 commits into from
Jan 18, 2021

Conversation

@vedanshujain
Copy link
Contributor

@vedanshujain vedanshujain commented Jan 14, 2021 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

In #28624 we fixed the root cause for $27688, however, we still needed a migration to fix the product review counts. This PR adds that migration, to fix the counts, we loop over all meta props with keys _wc_review_count and update them if they are incorrect and post belongs to a product.

Closes #27688.

How to test the changes in this Pull Request:

  1. Run this query to invalidate all product review count in your shop:
UPDATE %postmeta_table_name%
SET meta_value = '9999'
WHERE meta_key = '_wc_review_count';
  1. Go to any product page in the frontend and check that the review count is incorrect.
  2. Open a WP shell and run this command:
include_once dirname( WC_PLUGIN_FILE ) . '/includes/wc-update-functions.php';
WC_Install::run_update_callback('wc_update_500_fix_product_review_count'); // Keep on calling this till output is false.
  1. Go back to the product page in the frontend, refresh and check that the review count is now correct.

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 - Add migration to fix existing product review count.

@vedanshujain vedanshujain added this to the 5.0.0 milestone Jan 14, 2021
@vedanshujain vedanshujain requested review from a team and roykho and removed request for a team January 15, 2021 14:59
roykho
roykho reviewed Jan 15, 2021
View reviewed changes
Copy link
Contributor

@roykho roykho left a comment

Choose a reason for hiding this comment

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

Tested well for me. I had to checkout release/4.8 and followed the steps in the original issue to reproduce it. Then I checkout this PR and followed the testing steps and it fixed the arbitrary review count 9999 to the correct count in the product frontend.

Thanks for including the test cases. I just left small comments.

includes/class-wc-comments.php Outdated
$product_id_string = implode( "','", array_map( 'esc_sql', $product_ids ) );

$review_counts = $wpdb->get_results(
// phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared
Copy link
Contributor

@roykho roykho Jan 15, 2021

Choose a reason for hiding this comment

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

How come we're bypassing prepare here?

Copy link
Contributor Author

@vedanshujain vedanshujain Jan 18, 2021

Choose a reason for hiding this comment

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

This is because of $product_id_string which we were expanding for IN clause. I thought this is the standard way to use when doing interpolation for IN clause.

But after your comment, I looked in detail and see how WP core is doing it, and found this reference: https://github.com/pantheon-systems/wordpress-network/blob/master/wp-includes/class-wp-meta-query.php#L639.
This approach still ignores phpcs, but does not bypass prepare so I have changed the code to use this approach in 9f9475a. Let me know if this makes sense.

@vedanshujain
Addressed PR feedback.
9f9475a 
1: Use placeholder to be able to use wpdb->prepare for IN query.

Update version number.
roykho
roykho approved these changes Jan 18, 2021
View reviewed changes
Copy link
Contributor

@roykho roykho left a comment

Choose a reason for hiding this comment

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

LGTM!

@roykho roykho merged commit 56e67b1 into master Jan 18, 2021
@roykho roykho deleted the fix/migration/27688 branch January 18, 2021 14:48
@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 Jan 18, 2021
@roykho roykho removed the release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] label Jan 18, 2021
@claudiosanches claudiosanches added the release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] label Jan 18, 2021
@dennisheiden dennisheiden mentioned this pull request Mar 22, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@roykho roykho roykho approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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]

Projects

None yet

Milestone

5.0.0

Development

Successfully merging this pull request may close these issues.

[GlobalStep - WooCommerce 4.5.2] On front-end, Review count fails to update when user adds a review from WooCommerce dashboard.

5 participants

@vedanshujain @roykho @claudiosanches @woocommercebot