Show promo for setting up Google Ads on the product feed tabs #2539

[kt-12]

Changes proposed in this Pull Request:

Closes #2539

Replace this with a good description of your changes & reasoning.

Screenshots:

When there is Product but no campaign: ( Show the notice )

When there is Product and a campaign: ( Don't show the notice )

Detailed test instructions:

1. Create a Simple Product, sync to merchant account.
2. Visit Dashboard > Product Feed
3. If there is one active ( approved or partially approved) product and no campaign, notice will be shown. Check screenshots.
4. Click on the button and add campaign. Once added revisit Product Feed page and this section should disappear.

Additional details:

Further more this ticket address a bug in useAdsCampaign #2539 (comment), which required certain E2E to be adjusted.

Changelog entry

Add - Show promotion for Google Ads campaign on the Product Feed page.

[codecov]

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 63.5%. Comparing base (3553804) to head (616756f).
Report is 195 commits behind head on feature/2460-google-ads-value-prop.

Files with missing lines	Patch %	Lines
...product-statistics/create-campaign-notice/index.js	77.8%	2 Missing ⚠️

Additional details and impacted files

@@                           Coverage Diff                           @@
##             feature/2460-google-ads-value-prop   #2641      +/-   ##
=======================================================================
- Coverage                                  65.1%   63.5%    -1.6%     
=======================================================================
  Files                                       800     331     -469     
  Lines                                     24300    5214   -19086     
  Branches                                   1244    1273      +29     
=======================================================================
- Hits                                      15831    3313   -12518     
+ Misses                                     8287    1723    -6564     
+ Partials                                    182     178       -4     

Flag	Coverage Δ
js-unit-tests	63.5% <77.8%> (+1.5%)	⬆️
php-unit-tests	?

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

Files with missing lines	Coverage Δ
js/src/product-feed/product-statistics/index.js	95.7% <ø> (ø)
...product-statistics/create-campaign-notice/index.js	77.8% <77.8%> (ø)

... and 498 files with indirect coverage changes

[joemcgill]

@kt-12 I think I figured out some of the E2E issues and pushed a couple of commits. I've left a few other bits of feedback inline.

[asvinb]

@kt-12 I left a couple of comments. Can you kindly check them out please? Also, the unit tests are failing. Can you take a look?

[ankitguptaindia]

QA/Test Report- ✅

Testing Environment -

• WordPress: 6.6.2
• Theme active on store: Twenty Twenty-Four Version: 1.2
• WooCommerce - Version 9.3.2
• PHP: 8.3
• Web Server: Nginx
• Browser: Chrome - Version 128
• OS: macOS Sonoma 14.6.1

Test Results -

When No Paid Campaign available -

When Paid Campaign Available -

Paid campaign creation flow from Notice button:

Recording.914.mp4

[eason9487]

Further more this ticket address a bug in useAdsCampaign #2539 (comment), [...]

There is an existing issue with useAdsCampaigns that was discovered in #2242 (comment), where the hook will not fetch existing Ads accounts in certain circumstances.

I wouldn't consider it's a bug. Please refer to #2242 (comment). In terms of the requirement, does useAdsCampaigns need to be changed?

In addition, the e2e testing couldn't pass: https://github.com/woocommerce/google-listings-and-ads/actions/runs/11477389417/job/31942026350

[joemcgill]

@eason9487 thanks for the review, we're working through the feedback. Regarding this:

In terms of the requirement, does useAdsCampaigns need to be changed?

I do think that useAdsCampaigns needs to be changed for this requirement.

The purpose of this issue is to encourage merchants to create their first campaign but only if they have approved products and do not already have any ads campaigns created. To verify whether ads campaigns are running we need to check for existing campaigns on the account, which is what this hook seems to be for. However, if someone connects an existing account that already has ads and finishes onboarding by skipping creating a new ad, then the request to see if the account already has ads will never run.

This can be verified by connecting an account and creating a campaign.

Then from the settings screen, disconnect the account and reconnect it through the setup-ads flow and close without creating a new campaign. At this point, no more campaigns will show on the dashboard because no ads will be returned from the useAdsCampaigns hook, even though the account does have campaigns running.

This does not seem like expected behavior to me.

[eason9487]

Hi @joemcgill

I do think that useAdsCampaigns needs to be changed for this requirement.

Other side effects from changing useAdsCampaigns will need to be reviewed one by one to see if they need to be adjusted together, otherwise there may be inconsistent presentation at a high level.

For example, the campaign list is displayed on the Dashboard page, but the Performance card on the same page still doesn't show campaign performance data.

It's also possible that a merchant has connected to a Google Ads account with existing campaigns but has not completed the Ads Setup flow for a long time. The adsSetupComplete will be always false and ADS_SETUP_COMPLETED_AT doesn't exist. The frontend and backend logic associated with these two values will also be affected and may need to be reviewed one by one as well.

[joemcgill]

Other side effects from changing useAdsCampaigns will need to be reviewed one by one to see if they need to be adjusted together, otherwise there may be inconsistent presentation at a high level.

That makes sense. I've reviewed the codebase and am documenting my observations here for posterity.

It looks like all of the functionality that depends on the gla.adsSetupComplete global, including the useAdsCampaigns() hook assume that there are no campaigns to display until after the Ads account has been fully set up via the onboarding process or the secondary "Set up paid ads" flow. In both cases, "fully set up" means that an ad campaign has been created (not just that the Ads account has been connected).

The only way that global is getting set is firing the useAdsSetupCompleteCallback() hook here and here. Which make a POST request to the ads/setup/complete endpoint, resulting in the gla_ads_setup_completed_at option in the DB being set. Then, on a new page load, that option is checked to set the global via an inline script set here.

This means that as far as the plugin is concerned, you're not able to see and manage campaigns until you've created a new campaign after connecting the account (even if you reconnect an account you were previously using).

That limitation should probably be resolved, IMO, but that is well beyond the scope of what we're trying to solve here or in #2538. So for consistency, I think we'll keep useAdsCampaigns() hook as it was prior to our changes and make our new functionality for these issues work from the same assumption—i.e., that no campaigns are known until after a new campaign is created after connecting the Ads account.

I can log a new issue for the broader updates that we can look at coming back to.

[kt-12]

@eason9487 useAdsCampaigns mainly relies on the value of adsSetupComplete to make a campaign query. I tried using mockCampaignCreationAndAdsSetupCompletion on this page, but it didn't work here. Finally, I went ahead modifying the global as soon as it was available.

Let me know if there is a better approach here.

[eason9487]

A more suitable solution is to set ADS_SETUP_COMPLETED_AT by referring to the way of setting MC_SETUP_COMPLETED_AT:

• google-listings-and-ads/tests/e2e/test-data/test-data.php

  Lines 38 to 53 in 65972ac

  	register_rest_route(
  	'wc/v3',
  	'gla-test/onboarded-merchant',
  	[
  	[
  	'methods' => 'POST',
  	'callback' => __NAMESPACE__ . '\set_onboarded_merchant',
  	'permission_callback' => __NAMESPACE__ . '\permissions',
  	],
  	[
  	'methods' => 'DELETE',
  	'callback' => __NAMESPACE__ . '\clear_onboarded_merchant',
  	'permission_callback' => __NAMESPACE__ . '\permissions',
  	],
  	],
  	);

• google-listings-and-ads/tests/e2e/test-data/test-data.php

  Lines 72 to 101 in 65972ac

  	/**
  	* Set the onboarded merchant options.
  	*/
  	function set_onboarded_merchant() {
  	/** @var OptionsInterface $options */
  	$options = woogle_get_container()->get( OptionsInterface::class );
  	$options->update(
  	OptionsInterface::REDIRECT_TO_ONBOARDING,
  	'no'
  	);
  	$options->update(
  	OptionsInterface::MC_SETUP_COMPLETED_AT,
  	1693215209
  	);
  	$options->update(
  	OptionsInterface::GOOGLE_CONNECTED,
  	true
  	);
  	}
  	/**
  	* Clear a previously set onboarded merchant.
  	*/
  	function clear_onboarded_merchant() {
  	/** @var OptionsInterface $options */
  	$options = woogle_get_container()->get( OptionsInterface::class );
  	$options->delete( OptionsInterface::REDIRECT_TO_ONBOARDING );
  	$options->delete( OptionsInterface::MC_SETUP_COMPLETED_AT );
  	$options->delete( OptionsInterface::GOOGLE_CONNECTED );
  	}

• google-listings-and-ads/tests/e2e/utils/api.js

  Lines 102 to 114 in 65972ac

  	/**
  	* Set Onboarded Merchant.
  	*/
  	export async function setOnboardedMerchant() {
  	await api().post( 'gla-test/onboarded-merchant' );
  	}
  	/**
  	* Clear Onboarded Merchant.
  	*/
  	export async function clearOnboardedMerchant() {
  	await api().delete( 'gla-test/onboarded-merchant' );
  	}

After then, use them like this test file, and make sure to clear it in the test.afterAll block.

[kt-12]

Done this way.

[eason9487]

A more suitable solution is to set ADS_SETUP_COMPLETED_AT by referring to the way of setting MC_SETUP_COMPLETED_AT:

• google-listings-and-ads/tests/e2e/test-data/test-data.php

  Lines 38 to 53 in 65972ac

  	register_rest_route(
  	'wc/v3',
  	'gla-test/onboarded-merchant',
  	[
  	[
  	'methods' => 'POST',
  	'callback' => __NAMESPACE__ . '\set_onboarded_merchant',
  	'permission_callback' => __NAMESPACE__ . '\permissions',
  	],
  	[
  	'methods' => 'DELETE',
  	'callback' => __NAMESPACE__ . '\clear_onboarded_merchant',
  	'permission_callback' => __NAMESPACE__ . '\permissions',
  	],
  	],
  	);

• google-listings-and-ads/tests/e2e/test-data/test-data.php

  Lines 72 to 101 in 65972ac

  	/**
  	* Set the onboarded merchant options.
  	*/
  	function set_onboarded_merchant() {
  	/** @var OptionsInterface $options */
  	$options = woogle_get_container()->get( OptionsInterface::class );
  	$options->update(
  	OptionsInterface::REDIRECT_TO_ONBOARDING,
  	'no'
  	);
  	$options->update(
  	OptionsInterface::MC_SETUP_COMPLETED_AT,
  	1693215209
  	);
  	$options->update(
  	OptionsInterface::GOOGLE_CONNECTED,
  	true
  	);
  	}
  	/**
  	* Clear a previously set onboarded merchant.
  	*/
  	function clear_onboarded_merchant() {
  	/** @var OptionsInterface $options */
  	$options = woogle_get_container()->get( OptionsInterface::class );
  	$options->delete( OptionsInterface::REDIRECT_TO_ONBOARDING );
  	$options->delete( OptionsInterface::MC_SETUP_COMPLETED_AT );
  	$options->delete( OptionsInterface::GOOGLE_CONNECTED );
  	}

• google-listings-and-ads/tests/e2e/utils/api.js

  Lines 102 to 114 in 65972ac

  	/**
  	* Set Onboarded Merchant.
  	*/
  	export async function setOnboardedMerchant() {
  	await api().post( 'gla-test/onboarded-merchant' );
  	}
  	/**
  	* Clear Onboarded Merchant.
  	*/
  	export async function clearOnboardedMerchant() {
  	await api().delete( 'gla-test/onboarded-merchant' );
  	}

After then, use them like this test file, and make sure to clear it in the test.afterAll block.

[eason9487]

Thanks for the work and adjustments.

There are some suggestions for e2e tests and I believe it won't need a new round code review for these adjustments. I'll be approving it in advance.