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

Add marketplace theme discover section #40389

Conversation

bgrgicak
Copy link
Contributor

@bgrgicak bgrgicak commented Sep 25, 2023

Changes proposed in this Pull Request:

This PR is part of #40159

This PR adds support for theme sections on the Discover tab.
It also displays the new section on no-results for search theme.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Prep

To test the request from your local WooCommerce core wp-env environment to your local WCCOM environment, you first need a running instance of WCCOM.

Make sure you have the latest version of trunk in your WCCOM environment. Set up a group of three or four theme products in the Featured Products ad admin. (See the test steps in 18244-gh-Automattic/woocommerce.com for more details.) Check that the group is being returned from the featured endpoint https://woocommerce.test/wp-json/wccom-extensions/2.0/featured?country=US. (You'll probably need to flush the transient that caches this on your WCCOM with wp cache flush.)

Add a mu-plugin to wp-env (as suggested in p6JqRr-aWb), and put these two filters in it:

add_filter( 'http_request_host_is_external', '__return_true' );

add_filter( 'http_request_args', function ( $r ) {
	$r['reject_unsafe_urls'] = false;
	$r['sslverify']          = false;
	$r['timeout']            = 60;

	return $r;
} );

Get the ID of wp-env's WordPress container.

image

Link the container to woocommerce.test in the Docker network.

docker network connect woocommerce.test <container ID>

Test

  1. Until we get WCCOM API support, we need to point the API to local WCCOM with data available in the API
  2. Replace this line with:
$featured = false;
  1. Replace these lines with:
$raw_featured = wp_safe_remote_get(
				'https://woocommerce.test/wp-json/wccom-extensions/2.0/featured' . $parameter_string . '&no_cache=true',
  1. Open the Discover page
  2. Confirm that the Our favorite ecommerce themes. section is loaded
  3. Cards in this section should be displayed as themes

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

Add support for themes to marketplace discovery and use it in the theme no-result section.

Screenshot 2023-09-25 at 13 31 29

@bgrgicak bgrgicak self-assigned this Sep 25, 2023
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Sep 25, 2023
@bgrgicak bgrgicak changed the base branch from trunk to add/17702-implement-category-filtering September 25, 2023 10:36
@bgrgicak bgrgicak changed the base branch from add/17702-implement-category-filtering to update/in-app-multiple-category-filters September 25, 2023 10:36
@bgrgicak bgrgicak marked this pull request as ready for review September 25, 2023 11:55
@bgrgicak bgrgicak requested review from a team, Dan-Q and andfinally and removed request for a team September 25, 2023 11:55
@bgrgicak bgrgicak added focus: react marketplace Issues relating to the React Marketplace team: Fire labels Sep 25, 2023
@github-actions
Copy link
Contributor

Hi @Dan-Q, @andfinally,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

1 similar comment
@github-actions
Copy link
Contributor

Hi @Dan-Q, @andfinally,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@bgrgicak bgrgicak changed the title Add/marketplace theme discover section Add marketplace theme discover section Sep 25, 2023
@andfinally
Copy link
Contributor

andfinally commented Sep 27, 2023

Hm, I'm getting cURL error 7: Failed to connect to woocommerce.test port 443: Connection refused from the API when I try to load the page. These are my testing changes.

Index: plugins/woocommerce/includes/admin/class-wc-admin-addons.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/plugins/woocommerce/includes/admin/class-wc-admin-addons.php b/plugins/woocommerce/includes/admin/class-wc-admin-addons.php
--- a/plugins/woocommerce/includes/admin/class-wc-admin-addons.php	(revision 48e9566c2de2db0be72d175758aeceeb4e352add)
+++ b/plugins/woocommerce/includes/admin/class-wc-admin-addons.php	(date 1695815757702)
@@ -80,7 +80,7 @@
 	 */
 	public static function fetch_featured() {
 		$locale   = get_user_locale();
-		$featured = self::get_locale_data_from_transient( 'wc_addons_featured', $locale );
+		$featured = false; // self::get_locale_data_from_transient( 'wc_addons_featured', $locale );
 
 		if ( false === $featured ) {
 			$headers = array();
@@ -97,8 +97,8 @@
 			}
 
 			// Important: WCCOM Extensions API v2.0 is used.
-			$raw_featured = wp_safe_remote_get(
-				'https://woocommerce.com/wp-json/wccom-extensions/2.0/featured' . $parameter_string,
+			$raw_featured = wp_remote_get(
+			'https://woocommerce.test/wp-json/wccom-extensions/2.0/featured' . $parameter_string . '&no_cache=true',
 				array(
 					'headers'    => $headers,
 					'user-agent' => 'WooCommerce/' . WC()->version . '; ' . get_bloginfo( 'url' ),

The URL should be as shown, because $parameter_string starts with ?. For some reason, even with this change I'm getting Connection refused. The URL is accessible directly in my browser. 😞

Copy link
Contributor

@andfinally andfinally left a comment

Choose a reason for hiding this comment

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

I haven't been able to test against my local API, but the code looks good to me.

@bgrgicak
Copy link
Contributor Author

@andfinally could this be a networking issue?
Can you try connecting your Woo container (wordpress-1) to WC.test?

docker network connect woocommerce.test <CONTAINER-ID>

@andfinally
Copy link
Contributor

andfinally commented Sep 28, 2023

Thanks Bero, that didn't help.

  • Also tested with Local by Flywheel environment – no luck there either.
  • On debugging, it turned out the URL was being rejected because it appeared to be local. So I added this to my mu-plugin to allow it.
add_filter( 'http_request_host_is_external', '__return_true' );
  • However, now I'm getting cURL error 60: SSL certificate problem: unable to get local issuer certificate. The certificate authority is present in my browser, but I suppose it needs to be present on the container. 😭

LATER

Eureka! I just needed this filter as well. Adding to the test steps.

add_filter( 'http_request_args', function ( $r ) {
	$r['reject_unsafe_urls'] = false;
	$r['sslverify']          = false;
	$r['timeout']            = 60;

	return $r;
} );

…x-start` – items were looking weird centred when there weren't enough to fill a row.
Copy link
Contributor

@andfinally andfinally left a comment

Choose a reason for hiding this comment

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

Woooh this has been a marathon! Finally worked out that I needed both the filters and the Docker network connection for this to work. Also that you need some featured themes set up in the ad system. I've updated the test steps. I'm now seeing the new themes group as expected.

I made a tiny CSS change in 3d4841a to address this funny-looking centering when there aren't enough items to fill a row:

image

@github-actions
Copy link
Contributor

github-actions bot commented Sep 28, 2023

Test Results Summary

Commit SHA: 486706a

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 53s
E2E Tests211017021924m 26s

⚠️ Warning

Please address the following issues prior to merging this pull request:
  • FAILED/BROKEN TESTS. There were failed and/or broken API and E2E tests.

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@bgrgicak bgrgicak changed the base branch from update/in-app-multiple-category-filters to feature/marketplace-themes September 29, 2023 04:18
@bgrgicak
Copy link
Contributor Author

has has been a marathon! Finally worked out that I needed both the filters and the Docker network connection for this to work.

Thanks for taking the time @andfinally and sorry about the instructions. I probably configured some of these things over time and forgot about it.

@bgrgicak bgrgicak changed the base branch from feature/marketplace-themes to update/in-app-multiple-category-filters September 29, 2023 07:49
github-actions and others added 2 commits September 29, 2023 07:56
…ommerce/product-editor, @woocommerce/dependency-extraction-webpack-plugin, @woocommerce/data, @woocommerce/components, @woocommerce/block-templates, woocommerce-beta-tester, woocommerce
@github-actions github-actions bot added package: @woocommerce/components issues related to @woocommerce/components package: @woocommerce/data issues related to @woocommerce/data package: dependency-extraction-webpack-plugin issues related to @woocommerce/dependency-extraction-webpack-plugin plugin: woocommerce beta tester Issues related to the WooCommerce Beta Tester plugin. labels Sep 29, 2023
@Dan-Q
Copy link
Contributor

Dan-Q commented Sep 29, 2023

Sorry I didn't see your comment earlier @andfinally: as soon as I saw it, I thought "that's a certificate problem"! Glad you sussed it!

Copy link
Contributor

@Dan-Q Dan-Q left a comment

Choose a reason for hiding this comment

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

Tested using local WCCOM; looks good!

@andfinally andfinally merged commit 013116d into update/in-app-multiple-category-filters Sep 29, 2023
@andfinally andfinally deleted the add/marketplace-theme-discover-section branch September 29, 2023 16:40
@andfinally andfinally mentioned this pull request Oct 4, 2023
11 tasks
andfinally added a commit that referenced this pull request Oct 4, 2023
alopezari pushed a commit that referenced this pull request Oct 4, 2023
* Support for themes in in-app marketplace.

Contains the changes from:

#40247
#40272
#40302
#40303
#40333
#40368
#40375
#40375
#40389

* `.woocommerce-marketplace__discover`: changed `align-items` `flex-start` to `stretch` to properly display products on large and very large viewports.

* Delete plugins/woocommerce/changelog/add-18026-marketplace-theme-cards

Removing from feature branch before final review

* Delete plugins/woocommerce/changelog/add-18027-themes-to-in-app-search

Removing from feature branch before final review

* Delete plugins/woocommerce/changelog/add-marketplace-theme-discover-section

Removing from feature branch before final review

* Delete plugins/woocommerce/changelog/update-in-app-multiple-category-filters

Removing from feature branch before final review

* Delete plugins/woocommerce/changelog/update-theme-no-result-style

Removing from feature branch before final review

* Add changefile(s) from automation for the following project(s): woocommerce

---------

Co-authored-by: And Finally <andfinally@users.noreply.github.com>
Co-authored-by: Dan Q <dan@danq.me>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Dan Q <danq@automattic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: react marketplace Issues relating to the React Marketplace package: dependency-extraction-webpack-plugin issues related to @woocommerce/dependency-extraction-webpack-plugin package: @woocommerce/components issues related to @woocommerce/components package: @woocommerce/data issues related to @woocommerce/data plugin: woocommerce beta tester Issues related to the WooCommerce Beta Tester plugin. plugin: woocommerce Issues related to the WooCommerce Core plugin. team: Fire
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants