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

Fix Issue with intermediate parents status #2263

Conversation

jorgemd24
Copy link
Contributor

@jorgemd24 jorgemd24 commented Feb 20, 2024

Changes proposed in this Pull Request:

Follow-up of #2257.

Now that we're calculating product statuses in batches, it's possible for variations from the same parents to come in different batches. For example:

Batch 1: Variable 1 - Parent ID 1 - DISAPPROVED
Batch 2: Variable 2 - Parent ID 1 - APPROVED

This could impact the total count because variable products are grouped by their parents. So, in the previous example, we should only count 1 DISAPPROVED product instead of 1 DISAPPROVED + 1 APPROVED.

This PR addresses the issue by storing the latest status from the parent product, ensuring it can be used for the total calculation.

Detailed test instructions:

  1. It's a bit of a tricky situation to test, so I mainly relied on the Unit Tests to simulate the scenario. However, you can create batches of 1, each with a parent product having different variables, and check if the total count is correct. To do so identify a variable product in your store, and get a couple of variable ids with the same parent id.
  2. Check that those variables are available in your Merchant Center.
  3. Go to
    public function __construct( array $args ) {
    parent::__construct( 'ProductView' );
    $this->set_initial_columns();
    $this->handle_query_args( $args );
    }
    and add the following line: $this->where( 'product_view.offer_id', ['gla_VARIABLE_ID_1', 'gla_VARIABLE_ID_2'], 'IN' );
  4. Change the batch size to 1 in this line:
    $batch_size = apply_filters( 'woocommerce_gla_product_view_report_page_size', 500 );
  5. Call the following endpoint GET wc/gla/mc/product-statistics/refresh
  6. You should get only one status.

Changelog entry

@github-actions github-actions bot added changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug. labels Feb 20, 2024
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6a9a0f9) 59.9% compared to head (d7ecd99) 60.0%.
Report is 11 commits behind head on feature/refactor-product-stats.

Additional details and impacted files

Impacted file tree graph

@@                       Coverage Diff                        @@
##             feature/refactor-product-stats   #2263   +/-   ##
================================================================
  Coverage                              59.9%   60.0%           
- Complexity                             4178    4181    +3     
================================================================
  Files                                   457     457           
  Lines                                 17708   17721   +13     
================================================================
+ Hits                                  10611   10627   +16     
+ Misses                                 7097    7094    -3     
Flag Coverage Δ
php-unit-tests 60.0% <100.0%> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/MerchantCenter/MerchantStatuses.php 52.1% <100.0%> (+2.1%) ⬆️

... and 1 file with indirect coverage changes

@jorgemd24 jorgemd24 self-assigned this Feb 20, 2024
Base automatically changed from add/replace-wp-posts-functions-with-wc-objects to feature/refactor-product-stats February 20, 2024 15:39
@jorgemd24 jorgemd24 marked this pull request as ready for review February 20, 2024 19:21
@jorgemd24 jorgemd24 requested a review from a team February 20, 2024 19:21
Copy link
Contributor

@ianlin ianlin left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the problem of status priority in different batches. I tested using a small set of products and the calculation looked great. 👍

@jorgemd24 jorgemd24 merged commit 683c6d4 into feature/refactor-product-stats Feb 22, 2024
11 checks passed
@jorgemd24 jorgemd24 deleted the fix/issue-with-intermediate-parents-status branch February 22, 2024 17:19
@eason9487 eason9487 mentioned this pull request Mar 26, 2024
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants