Product Collection: Parse front-end context #44145

xristos3490 · 2024-01-28T20:45:58Z

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

This Pull Request parses the front-end location context for each Product Collection block and passes it to its context.

Context Levels

The context is parsed per block. This happens because it's easier to serve the following priorities:

Block-level context: A product selected in the (future) context selector in the inspector controls (Pending; probably in a different PR.)
Ancestor-level context: A product fetched by a possible Single Product container block or nested ones.
Global: Parses the global wp_query.

Requirements

Possible contexts:

Product
Cart/Checkout
Order
Archive
Site (generic, empty source data; the Shop page belongs here as well)

All need to be specific and fill in the required source data:

Product: The product ID.
Cart: The cart contents product IDs.
Archive: The taxonomy name and the viewed term ID.
Order: The order ID.

Note

By default, context is only retrieved in pages and entities related to WooCommerce. This means that if a post contains a product collection block or a non-related taxonomy like category, the context will be empty and default to site.

Closes #44144

How to test the changes in this Pull Request:

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

Before testing

Add the following PHP snippet to help with identifying the context:

add_filter(
	'render_block_woocommerce/product-template',
	function ( $block_content, $block, $instance ) {
		$location      = $instance->context['productCollectionLocation'] ?? array();
		$block_content = print_r( $location, true ) . $block_content;
		return $block_content;
	},
	0,
	3
);

Insert a product collection block on all pages, including single product templates, the shop page, order confirmation, single pages or posts, cart, and checkout.
Ensure that there is at least one global product attribute and a product using this attribute. To access archives, visit an attribute's edit page by navigating to Products > Attributes, edit an attribute, and check the "Enable Archives" option.

Testing

Start exploring.
Add a Product Collection block to the following places:

Shop
Single Products
Single Posts
Within a single Post.
Single Product: Template for a specific product.
Within a Page.
Within a Single Product container block.
Within a Pattern; add the pattern somewhere.
Within a Synced Pattern; add the pattern somewhere again.
Any other cases?

Notice the context on each case printed on top.

Negative testing

Ensure that only woo-related pages are getting a context value.
Test that a simple Post (or any other CPT) or the category tax archive returns the generic context.

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

github-actions · 2024-01-28T21:03:20Z

Test Results Summary

Commit SHA: a48b3ec

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 38s
E2E Tests	313	0	0	38	0	351	7m 10s

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.

github-actions · 2024-01-29T11:23:37Z

Hi @kmanijak,

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

imanish003

Hi @chli,
I've reviewed the code of this PR, and it looks good to me so far. However, I'm uncertain about conducting thorough testing since the PR hasn't covered all cases yet. Should I wait for my review until it fully addresses all cases? 🙂

imanish003 · 2024-02-06T11:32:47Z

Also, a quick note—I'm noticing some PHPCS warnings. It seems like there might be an issue with your PHPCS configuration 🤷‍♂️.

…context to collection children

imanish003

Hey @xristos3490, great work! 🙌

I believe we're almost at the finish line. I've left a couple of minor suggestions for you to consider.

plugins/woocommerce/src/Blocks/BlockTypes/ProductCollection.php

plugins/woocommerce/src/Blocks/Utils/ProductCollectionUtils.php

imanish003

Hey @xristos3490,

I've dropped a few minor comments. I plan to do a bit more testing later today but wanted to share this feedback with you now. Also, it would be wonderful if @kmanijak could also give it a quick review.

Thanks for the amazing work 🙌🏻

plugins/woocommerce/src/Blocks/BlockTypes/ProductCollection.php

plugins/woocommerce/src/Blocks/Utils/ProductCollectionUtils.php

imanish003 · 2024-03-06T08:52:07Z

plugins/woocommerce/src/Blocks/BlockTypes/ProductCollection.php

+		if ( 'woocommerce/product-collection' === $block['blockName']
+			&& isset( $block['attrs']['query']['isProductCollectionBlock'] ) ) {
+					$this->product_collection_inner_blocks_names = array_reverse(
+						$this->extract_inner_block_names( $block )
+					);
+		}
+
+		$this->provide_location_context_inner_blocks( $block, $context );


TBH I found this logic a bit hard to understand. Here is what I understood, please correct me if I am wrong.

For example, If I have "On Sale" collection:

Then, our logic depends on the fact that render_block_context will be executed in following sequence on Frontend:

Based on above fact, we push the items on Stack (product_collection_inner_blocks_names) & then start popping those items.

Question: Is it good idea to assume that flow of render_block_context hook will always be like this in future? Am I missing something?

Also, looks like we don't have any other good option because we don't have access to any unique ID. We just have access to "blockName".

CC: @thealexandrelara As you worked on implementing similar logic in woocommerce/woocommerce-blocks#8610

I understand that the current method of working with inner blocks may not be ideal, but it appeared to be functioning with the Single Product block. It's essential to consider that the inner blocks structure in a Product Collection can be anything and that other blocks, such as the no-results block, will also require access to that context, as you mentioned.

Upon reading this comment, it sparked some thoughts that led me to realize that there may be a problem with how the system operates. This issue impacts both the current case and the Single Product block. Upon further investigation, my assumptions proved to be accurate. I have provided a detailed explanation of this problem here:

[Single Product] Incomplete Context Passing in Nested Single Product Blocks #45342

I will work on developing a better plan to handle edge cases, and hopefully, I can also resolve the issue in the single product container.

Upon further testing, it seems the issue here is more severe than I anticipated. This is because every time the woocommerce/product-template block is rendered, it runs through every inner block for each individual product in the query results. As a result, the required execution sequence is being destroyed.

Back to the drawing board.

Update: The existence of a product-template block (either within a Query Loop or a Product Collection block) breaks the Single Product block as well for the same reason - #45342 (comment).

I'm glad that it sparked some thoughts and that you were able to identify the edge cases where this approach does not work as expected. It would be great if you could find a simpler and more effective solution to this 🙂

Does this mean that we cannot merge this PR until we have found a better solution?

Also, regarding #44145 (comment):

How do you feel about using Reflection to access available_context property? Something like this:

$reflector = new ReflectionObject( $parent_block ); $property = $reflector->getProperty( 'available_context' ); $property->setAccessible( true ); $available_context = $property->getValue( $parent_block );

CC: @thealexandrelara As you worked on implementing similar logic in woocommerce/woocommerce-blocks#8610

It's been a while since I last looked at that code 😅 . TBH, when the Single Product block was created, we didn't anticipate the possibility of it being nested within another Single Product block. I believe that if that's a valid use case, we could probably change the logic to support it. Each rendered block has a unique clientId, so perhaps we can use that identifier to differentiate between different block instances and apply the corresponding context for each nested block (I'm not sure if that is actually possible, so I'm just braintorming here).

IMO, It would be better to use isProductCollectionBlock instead. You may find some examples by searching isProductCollectionBlock in this file

@imanish003, It seems to me that using the query context is the best choice for this situation. If that's what you meant, then you're absolutely right!

After taking a closer look into how the block context works in PHP, it seems that utilizing WordPress's native context is the most efficient and straightforward approach to bypass the barrier of the protected $available_context.

This guarantees that the location context is only added within the context of blocks that use the "query" context and include the [query][isProductCollectionBlock] value, which helps identify the inner blocks.

I made these changes here: 17de651, and they seem to be working great! Can you take another look?

Thanks for chiming in, @thealexandrelara!

we didn't anticipate the possibility of it being nested within another Single Product block

💯 agree that this is an edge case!

For me, the main issue that this PR surfaced is the one I described in #45374. The product-template spawns new blocks during render, which are not accounted for in the block names execution stack, causing a break in the flow when adding the single product's context.

plugins/woocommerce/src/Blocks/BlockTypes/ProductCollection.php

Co-authored-by: Manish Menaria <the.manish.menaria@gmail.com>

kmanijak

Great work @xristos3490 !

Testing:

❌ in Cart - there's no productIds even though there are products in Cart

There's also PHP error:

plugins/woocommerce/src/Blocks/BlockTypes/ProductCollection.php

Parse global context

d51fafe

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jan 28, 2024

xristos3490 requested review from imanish003 and kmanijak January 29, 2024 11:23

imanish003 reviewed Feb 6, 2024

View reviewed changes

xristos3490 added 2 commits February 27, 2024 16:48

Merge branch 'trunk' into add/44144-front-end-context

b68d86e

Refactor the util to only parse global context, provide the location …

128f586

…context to collection children

xristos3490 changed the title ~~Product Collection: Parse front-end context (POC)~~ Product Collection: Parse front-end context Feb 28, 2024

xristos3490 added 2 commits February 28, 2024 17:13

cleanup

7251996

Cleanup

49425f9

xristos3490 marked this pull request as ready for review March 1, 2024 12:12

xristos3490 added 2 commits March 1, 2024 14:16

Merge branch 'trunk' into add/44144-front-end-context

7d9bc37

Add changelog

399f957

xristos3490 self-assigned this Mar 1, 2024

imanish003 requested changes Mar 5, 2024

View reviewed changes

xristos3490 added 3 commits March 6, 2024 02:24

Make linters happy

4cac5c1

provide context to each inner block recursively

5b11ae9

Fix linters

7c15f4e

imanish003 requested changes Mar 6, 2024

View reviewed changes

xristos3490 and others added 3 commits March 6, 2024 11:25

Remove debug

ec0a08f

Update plugins/woocommerce/src/Blocks/BlockTypes/ProductCollection.php

7dc54df

Co-authored-by: Manish Menaria <the.manish.menaria@gmail.com>

Rename the provider method

d94c2c2

kmanijak requested changes Mar 6, 2024

View reviewed changes

plugins/woocommerce/src/Blocks/BlockTypes/ProductCollection.php Outdated Show resolved Hide resolved

xristos3490 added 4 commits March 6, 2024 16:12

Fix typo on cart item products

43ebcc9

Use the query context instead

17de651

Lint

beb8ca6

Lint again

a48b3ec

kmanijak self-requested a review March 13, 2024 07:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Product Collection: Parse front-end context #44145

Product Collection: Parse front-end context #44145

xristos3490 commented Jan 28, 2024 •

edited

github-actions bot commented Jan 28, 2024 •

edited

github-actions bot commented Jan 29, 2024 •

edited

imanish003 left a comment

imanish003 commented Feb 6, 2024

imanish003 left a comment

imanish003 left a comment

imanish003 Mar 6, 2024

imanish003 Mar 6, 2024

xristos3490 Mar 6, 2024 •

edited

xristos3490 Mar 6, 2024 •

edited

imanish003 Mar 7, 2024

imanish003 Mar 7, 2024

thealexandrelara Mar 7, 2024

xristos3490 Mar 8, 2024

xristos3490 Mar 8, 2024

kmanijak left a comment

Product Collection: Parse front-end context #44145

Are you sure you want to change the base?

Product Collection: Parse front-end context #44145

Conversation

xristos3490 commented Jan 28, 2024 • edited

Submission Review Guidelines:

Changes proposed in this Pull Request:

Context Levels

Requirements

How to test the changes in this Pull Request:

Before testing

Testing

Negative testing

Changelog entry

Significance

Type

Message

Comment

github-actions bot commented Jan 28, 2024 • edited

Test Results Summary

github-actions bot commented Jan 29, 2024 • edited

imanish003 left a comment

Choose a reason for hiding this comment

imanish003 commented Feb 6, 2024

imanish003 left a comment

Choose a reason for hiding this comment

imanish003 left a comment

Choose a reason for hiding this comment

imanish003 Mar 6, 2024

Choose a reason for hiding this comment

imanish003 Mar 6, 2024

Choose a reason for hiding this comment

xristos3490 Mar 6, 2024 • edited

Choose a reason for hiding this comment

xristos3490 Mar 6, 2024 • edited

Choose a reason for hiding this comment

imanish003 Mar 7, 2024

Choose a reason for hiding this comment

imanish003 Mar 7, 2024

Choose a reason for hiding this comment

thealexandrelara Mar 7, 2024

Choose a reason for hiding this comment

xristos3490 Mar 8, 2024

Choose a reason for hiding this comment

xristos3490 Mar 8, 2024

Choose a reason for hiding this comment

kmanijak left a comment

Choose a reason for hiding this comment

xristos3490 commented Jan 28, 2024 •

edited

github-actions bot commented Jan 28, 2024 •

edited

github-actions bot commented Jan 29, 2024 •

edited

xristos3490 Mar 6, 2024 •

edited

xristos3490 Mar 6, 2024 •

edited