Refactor Account and Presync issues

[jorgemd24]

Changes proposed in this Pull Request:

Part of #2146 & #2250

This PR fetches the account and presync issues using the same background job used to fetch the product statuses. The issues will be fetched in the first batch. The logic remains mostly unchanged from before, the main difference is the addition of a new property `loading when retrieving the issues. This indicates whether the job is scheduled or running and if the process to fetch the issues is in progress. I'll create a follow-up PR to update the UI to reflect the loading status of the issues.

Detailed test instructions:

1. Set your PHP memory limit low so you can catch any heavy process. For example, setting this up to 128MB.
2. Call GET wp-json/wc/gla/mc/product-statistics/refresh
3. Go to WooCommerce -> Status -> Action Scheduler -> Search for gla/jobs/update_merchant_product_statuses/process_item see how the jobs get completed successfully.
4. See how the table wp_gla_merchant_issues gets populated with presync and account issues.

Additional details:

• I refactor maybe_refresh_status_data so it returns the AS job, so it is easier to determine if the process of updating the status and issues is scheduled or in progress.
• If the mc_statuses is not set for the product, we will consider that product is not synced in the MC. See here: Create product statuses jobs for each page token #2255 (comment)

Changelog entry

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.5%. Comparing base (c06f4e9) to head (6b7a614).
Report is 1 commits behind head on feature/refactor-product-stats.

Additional details and impacted files

@@                        Coverage Diff                         @@
##             feature/refactor-product-stats   #2298     +/-   ##
==================================================================
+ Coverage                              60.3%   60.5%   +0.2%     
- Complexity                             4181    4183      +2     
==================================================================
  Files                                   457     457             
  Lines                                 17733   17745     +12     
==================================================================
+ Hits                                  10693   10742     +49     
+ Misses                                 7040    7003     -37     

Flag	Coverage Δ
php-unit-tests	60.5% <100.0%> (+0.2%)	⬆️

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

Files	Coverage Δ
...te/Controllers/MerchantCenter/IssuesController.php	85.5% <100.0%> (+0.7%)	⬆️
src/DB/ProductFeedQueryHelper.php	53.9% <100.0%> (+15.1%)	⬆️
src/Jobs/UpdateMerchantProductStatuses.php	92.9% <100.0%> (+0.3%)	⬆️
src/MerchantCenter/MerchantStatuses.php	72.5% <100.0%> (+5.9%)	⬆️
src/Product/ProductHelper.php	95.4% <100.0%> (ø)

[puntope]

💅 use WP_REST_Response as Response;Is unused.

[puntope]

💅 What do you think about updating the return docs?

* @return array With two indices, results (may be paged) and count (considers type).

Now we are returning 'loading' as well

[puntope]

❓ Do we really need this chunk of code? Maybe we can keep statusas null when is refreshing.

[jorgemd24]

I think it is necessary because if there's no mc_status, it'll just return 'not-synced'. And most of the time, mc_status won't be null.

If the mc_statuses is not set for the product, we will consider that product is not synced in the MC. See here: #2255 (comment)

google-listings-and-ads/src/DB/ProductFeedQueryHelper.php

Line 100 in 6b7a614

$mc_status = $product_helper->get_mc_status( $product ) ?: $product_helper->get_sync_status( $product );

I switched from using - to null because it might be clearer to indicate that there's no status yet.

[puntope]

Why we need double-checking if the job is running?

AFAIK schedule() checks is_running() for the current page.

I see you created the is_Schedule function without arguments to avoid page checks?

public function is_scheduled(): bool {
		// We set 'args' to null so it matches any arguments. This is because it's possible to have multiple instances of the job running with different page tokens
		return $this->is_running( null );
	}

Maybe we can just tweak can_schedule function and then rely on $job->schedule itself to check if the job is running.

/**
	 * Can the job be scheduled.
	 *
	 * @param array|null $args
	 *
	 * @return bool Returns true if the job can be scheduled.
	 */
	public function can_schedule( $args = [] ): bool {
		return parent::can_schedule( $args ) && $this->merchant_center->is_connected() && ! $this->is_running( null );
	}

[jorgemd24]

Thanks @puntope for the suggestion, if we do it like that, the second job won't start because at the time we schedule it, the first job hasn't finished yet.

google-listings-and-ads/src/Jobs/UpdateMerchantProductStatuses.php

Line 101 in 6b7a614

$this->schedule( [ [ 'next_page_token' => $next_page_token ] ] );

[puntope]

💅 Maybe we can add @since param

[puntope]

Thanks @jorgemd24 for the refactor work. Everything works as expected. I left just some comments regarding the code.

[jorgemd24]

Thanks @puntope for your time and your suggestions! I addressed your comments and replied to your questions! Do you mind to have a second look? Thanks!

[puntope]

Thanks for changes and explanation @jorgemd24 Looking good to me.