woocommerce / woocommerce-blocks

(Deprecated) This plugin has been merged into woocommerce/woocommerce
https://wordpress.org/plugins/woo-gutenberg-products-block/
GNU General Public License v3.0
405 stars 218 forks source link

Password Protection Not Respected on Single Product Template #10939

Closed danielwrobert closed 1 year ago

danielwrobert commented 1 year ago

Describe the bug

When viewing a password protected Produce on a block-based theme, the password protection is not respected on the frontend and the product is revealed without requiring the password.

On a classic theme, however, the setting is respected.

Using the Classic single product template block for the Single product template it works fine, but as soon as it is converted into blocks the password protection rule is no longer applied on the frontend.

To reproduce

Steps to reproduce the behavior:

  1. Ensure that you have a product that is password protected. If not, set one to be protected via the Visibility setting in the admin (under the Publish metabox).
  2. Activate the Storefront theme.
  3. Visit the password protected product on the frontend.
  4. Confirm the product requires the password before showing the product page.
  5. Activate the TT3 theme.
  6. Visit the password protected product on the frontend.
  7. Confirm the product does not require the password before showing the product page.

Expected behavior

When a product is protected with a password, the front-end should respect that setting (i.e. it should ask for a password).

Screenshots

Classic Theme Block Theme
CleanShot 2023-09-13 at 17 03 36 CleanShot 2023-09-13 at 16 59 51

Environment

WordPress (please complete the following information):

+-------------------------------+----------+------------------------------+------------+
| name                          | status   | update                       | version    |
+-------------------------------+----------+------------------------------+------------+
| akismet                       | inactive | available                    | 5.2        |
| gutenberg                     | inactive | none                         | 16.6.0     |
| hello                         | inactive | none                         | 1.7.2      |
| jetpack                       | active   | none                         | 12.5       |
| smntcs-show-symlinked-plugins | active   | none                         | 1.1        |
| woocommerce                   | active   | none                         | 8.1.0      |
| woocommerce-blocks            | active   | version higher than expected | 11.2.0-dev |
| wordpress-importer            | active   | none                         | 0.8.1      |
+-------------------------------+----------+------------------------------+------------+

Desktop (please complete the following information):

Smartphone (please complete the following information):

Additional context

gigitux commented 1 year ago

Thanks for opening this issue! After some investigation, I found the issue, and there are several ways to fix this. Currently, we don't check if a password protects a product. When a product is protected, we need to display the HTML generated by the get_the_password_form() function.

I've outlined three possible solutions below:

Replace ALL the Single Product Template content

The simplest solution is to replace all the content of the template. This approach requires minimal effort (maximum 1 hour), but the final result may not be ideal. By replacing all the content of the template, the header and footer will not be visible:

Frontend Editor
image image

Remove all the blocks related to the Single Product Template

Another approach is to remove all blocks related to the Single Product Template from the template and inject the get_the_password_form HTML after the first template part of the page (which is typically the header). However, this method has a drawback. If a merchant has extended the Single Product Template for a specific product and added a paragraph block to provide additional product details, this block will not be affected by this approach and will still be displayed.

Crate a new block template (e.g. Single Product Template - protect view)

I believe this is the most favorable solution and it's not overly complex. We can create a dedicated template to handle the Protected Product page view. In the initial iteration, we can use a placeholder, similar to what we did for the Archive and Single Product pages. This way, merchants will have the flexibility to customize the page around the password form while keeping the form itself intact.

cc @nerrad @chrisdesrochers

tjcafferkey commented 1 year ago

Thanks for looking into this and presenting the above options @gigitux, great job. Lets wait for @nerrad and/or @chrisdesrochers input before moving forward but in my opinion it sounds like we can immediately implement the "Replace ALL the Single Product Template content" option (#1) which appears to be very low effort, with a view to replace this with a new block template (#3).

Any idea how much effort would be involved in creating a new block template for this view?

gigitux commented 1 year ago

Any idea how much effort would be involved in creating a new block template for this view?

I think a few hours is enough. Max 1 Day - 1 Day and a half. The main issue is to tweak this array to return as a first template the "Single Product Template - protect view" and avoid any regression.

nerrad commented 1 year ago

How are password-protected posts/pages in WP working right now?

gigitux commented 1 year ago

Post/Pages are handled by the function get_the_content that implements this check https://github.com/WordPress/wordpress-develop/blob/0bfc842f807582906bc119eb62aff6bd1c1e8dea/src/wp-includes/post-template.php/#L316-L320.

Editor Site Editor Frontend
image image image

In WooCommerce, this logic was handled by the content-single-product https://github.com/woocommerce/woocommerce/blob/64e3b4aaf633f063c3ea41086c631ef8375b78e2/plugins/woocommerce/templates/content-single-product.php/#L29-L32.

In the block context, we have multiple blocks that show product details, so we should implement the check for each block and find a way to render the input only once (#2 implementation).

nerrad commented 1 year ago

Gotcha, I guess what I was wondering is specific to this approach:

Another approach is to remove all blocks related to the Single Product Template from the template and inject the get_the_password_form HTML after the first template part of the page (which is typically the header). However, this method has a drawback. If a merchant has extended the Single Product Template for a specific product and added a paragraph block to provide additional product details, this block will not be affected by this approach and will still be displayed.

It seems that this would be the same case for custom single post/page templates as well right? In other words, this is an issue that exists for native post types in WordPress as well?

gigitux commented 1 year ago

Yes!

The check is done in the Post Content block.

This is what happens with a custom single post template:

Site Editor Frontend
image image
nerrad commented 1 year ago

Right, so at least it's consistent if we only do the password protection check on blocks that utilize product context for their content. I think the behaviour between template and content when it comes to protected content can be surfaced better upstream but at least this provides us a consistent path to follow for password protection set at the content level.

Does this help?

gigitux commented 1 year ago

I think the behaviour between template and content when it comes to protected content can be surfaced better upstream

Yeah. I'm going to open an issue on Gutenberg.

but at least this provides us a consistent path to follow for password protection set at the content level.

Yep, with #2, we will have the same behavior that exists for WordPress with posts/pages. Should we fix it ASAP or wait for the cooldown/next cycle?

nerrad commented 1 year ago

Yep, with #2, we will have the same behavior that exists for WordPress with posts/pages. Should we fix it ASAP or wait for the cooldown/next cycle?

I think we should try and fix it ASAP. While the fact it hasn't been reported yet points to the likelihood this feature isn't used much for Product post types, it still is something that we should make sure works as expected. I consider this high priority, not critical, and it doesn't require releasing outside of our normal release cycles.

I also don't think we need to prioritize doing a separate template for the form either. To me, that seems like something that should be offered at the WP core level.

gigitux commented 1 year ago

I created a PR that fixes the issue #10999. Futhermore, I created an issue on the Gutenberg repo: https://github.com/WordPress/gutenberg/issues/54607.