Add Auth token for search/featured extensions requests #28719

raicem · 2021-01-05T10:03:52Z

All Submissions:

Have you followed the WooCommerce Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Start using this endpoint to fetch featured extensions. Also add the access token to this request.
Add access token to the search request made to the this endpoint. Start using wp_safe_remote_get for it instead of wp_remote_get.
Reduce the transient duration for featured extensions to one day

How to test the changes in this Pull Request:

// Disable this transient to make sure you are hitting the new endpoint
add_filter('transient_wc_addons_featured', function ($value, $transient) {
	return false;
}, 10, 2);

When not authenticated to the WooCommerce.com go to the WooCommerce->Extensions page. The featured tab should work as before. Making a search on the marketplace should also work as before.
When authenticated to the WooCommerce.com, featured and search tabs should keep working the same. This time they should include the Authorization header containing the access_token.

// Inspect headers sent in the requests made to the WooCommerce.com
function action_http_api_debug( $response, $context, $class, $args, $url ) {
	if ( $context !== 'response' ) {
		return;
	}

	if ( strpos( $url, 'woocommerce.com' ) !== false ) {
		wc_get_logger()->debug( implode( ', ', $args['headers'] ) );
	}
}

add_action( 'http_api_debug', 'action_http_api_debug', 10, 5 );

Further testing for WooCommerce.com

If you have WooCommerce.com running locally and would like to test this feature you can use this repo to create a new store using this branch of WooCommerce plug-in.

Checkout this branch.
Update references to woocommerce.com to woocommerce.test on this file
Visit featured extensions and search request in your store
You should see the Authorization header info printed in the debug.log

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

Update featured extensions API endpoint on WooCommerce/Extensions page
Add Authorization header to the features extensions and search requests made to the marketplace

WooCommerce.com will provide this list of products.

The featured extension list was static but now it's fetched from WooCommerce.com and may change more frequently. Reducing the cache duration to allow users to see changes much more quickly.

bgrgicak · 2021-01-06T08:22:12Z

includes/admin/class-wc-admin-addons.php

+			$headers['Authorization'] = 'Bearer ' . $auth['access_token'];
+		}
+
 		$raw_extensions = wp_remote_get(


@raicem could we change this to wp_safe_remote_get?

I agree that's a good idea.

Ah, sorry. I have overlooked this. Fixed with this commit: eb3ba82

bgrgicak · 2021-01-06T08:29:00Z

includes/admin/class-wc-admin-addons.php

 				$featured = json_decode( wp_remote_retrieve_body( $raw_featured ) );
 				if ( $featured ) {
-					set_transient( 'wc_addons_featured', $featured, WEEK_IN_SECONDS );
+					set_transient( 'wc_addons_featured', $featured, DAY_IN_SECONDS );


@mattyza do you think that it’s ok to keep caching featured page content, or should we stop caching it?
Caching reduces the number of requests to WooCommerce.com, but it prevents us from making immediate changes.

Perhaps a job for another day, but it seems to me like the optimal approach would be:

Continue to cache for performance reasons, as well to increase the likelihood of seeing content if for some reason the site can't connect to WCCOM

Clear this cache on installation/uninstallation/activation/deactivation of a plugin, resync of connection to WCCOM, etc.: this way, if a customer buys a new plugin they're likely to see the same recommendations... but then if they install it their recommendations will get updated!

This makes sense @Dan-Q. Leaving the caching to last a day will keep the number of requests small but it will still allow us to change Algolia rules and to know that the changes will apply within 24 hours.

To keep cache clearing simple I suggest that we just add it to the subscriptions helper API call https://github.com/Automattic/woocommerce.com/blob/475ada19df47a07ac057a4fa1c207d129be1eb8a/plugins/woocommerce/includes/admin/helper/class-wc-helper.php#L1264

If we decide to make any changes, can we do them in a separate PR? To avoid delaying this PR.

Caching definitely makes sense, here. As Dan mentioned, having a way to invalidate this cache on demand is important. This can be done on disconnecting and reconnecting to WooCommerce.com, which would give Happiness Engineers a way to instruct customers to refresh their cache if needed.

There are, I'm sure, other options which we could explore in future PRs.

Caching for 1 day seems reasonable to me.

bgrgicak

Thanks @raicem! This looks great and works as expected 🎉

Dan-Q

Looks good!

It might have been nice to avoid repetition by refactoring-out the duplicated bits, but I suggest that waits until we inevitably end up adding authorization tokens to more requests. ;-)

(Also: I wonder if down the line we might want some hooks associated with the get_extension_data and get_featured requests to make adding tokens more-modular.)

Anyway, this looks good to me, for sure.

raicem · 2021-01-12T19:44:40Z

I have removed marketplace_base_url filter based upon the feedback from @claudiosanches. Thanks Claudio for taking a look.

claudiosanches

@raicem great work!
Everything seems to be working as expected.

Could you please address this change: https://github.com/woocommerce/woocommerce/pull/28719/files#r552430798
Thanks!

claudiosanches · 2021-01-13T20:38:32Z

includes/admin/class-wc-admin-addons.php

+			$headers['Authorization'] = 'Bearer ' . $auth['access_token'];
+		}
+
 		$raw_extensions = wp_remote_get(


I agree that's a good idea.

claudiosanches

Great work!

raicem added 3 commits January 5, 2021 12:43

Update featured products endpoint

c628c15

WooCommerce.com will provide this list of products.

Send auth token when sending search and featured extension requests

c70eee2

Reduce "wc_addons_featured" transient cache duration

7b67977

The featured extension list was static but now it's fetched from WooCommerce.com and may change more frequently. Reducing the cache duration to allow users to see changes much more quickly.

raicem changed the title ~~Update featured extensions API endpoint~~ Add Auth token for search/featured extensions requests Jan 5, 2021

bgrgicak reviewed Jan 6, 2021

View reviewed changes

Fix indentation errors as per WordPress code style guides

f0166fc

raicem requested review from Dan-Q and peterfabian January 7, 2021 13:56

raicem self-assigned this Jan 7, 2021

raicem added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Jan 7, 2021

raicem marked this pull request as ready for review January 7, 2021 13:57

bgrgicak previously approved these changes Jan 8, 2021

View reviewed changes

Dan-Q previously approved these changes Jan 12, 2021

View reviewed changes

claudiosanches added this to the 5.0.0 milestone Jan 12, 2021

raicem dismissed stale reviews from Dan-Q and bgrgicak via f0166fc January 12, 2021 19:36

claudiosanches suggested changes Jan 13, 2021

View reviewed changes

Use "wp_safe_remote_get" for sending search requests

eb3ba82

claudiosanches approved these changes Jan 13, 2021

View reviewed changes

claudiosanches removed the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Jan 13, 2021

claudiosanches merged commit 1f321f4 into woocommerce:master Jan 13, 2021

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 13, 2021

claudiosanches added release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Jan 13, 2021

Add Auth token for search/featured extensions requests #28719

Add Auth token for search/featured extensions requests #28719

Uh oh!

Conversation

raicem commented Jan 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Changelog entry

Uh oh!

bgrgicak Jan 6, 2021

Choose a reason for hiding this comment

Uh oh!

claudiosanches Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raicem Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

bgrgicak Jan 6, 2021

Choose a reason for hiding this comment

Uh oh!

Dan-Q Jan 12, 2021

Choose a reason for hiding this comment

Uh oh!

bgrgicak Jan 12, 2021

Choose a reason for hiding this comment

Uh oh!

bgrgicak Jan 12, 2021

Choose a reason for hiding this comment

Uh oh!

mattyza Jan 12, 2021

Choose a reason for hiding this comment

Uh oh!

bgrgicak left a comment

Choose a reason for hiding this comment

Uh oh!

Dan-Q left a comment

Choose a reason for hiding this comment

Uh oh!

raicem commented Jan 12, 2021

Uh oh!

claudiosanches left a comment

Choose a reason for hiding this comment

Uh oh!

claudiosanches Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claudiosanches left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

raicem commented Jan 5, 2021 •

edited

Loading

claudiosanches Jan 13, 2021 •

edited

Loading

claudiosanches Jan 13, 2021 •

edited

Loading