Skip to content

Conversation

@mattsherman
Copy link
Contributor

@mattsherman mattsherman commented Sep 20, 2022

All Submissions:

Changes proposed in this Pull Request:

In WooCommerce 6.9, a tip was added under the product image thumbnail in the sidebar of the product edit page (see #33660).

Under certain circumstances, this tip can be rendered multiple times.

Screen Shot 2022-09-19 at 19 56 17

This is because the add_filter call is incorrectly called from the constructor of the WC_Simple_Product class. This is not a good location to call add_filter because:

  1. There is no guarantee that a plugin or even core functionality will not construct multiple WC_Simple_Product instances while rendering the product edit page, and
  2. By calling it from the WC_Simple_Product constructor, it doesn't appear for other product types, such as variable products.

I've changed the implementation to work when editing any product type and to only render a single time.

Screen Shot 2022-09-19 at 19 51 16

Fixes #34728.

How to test the changes in this Pull Request:

  1. Add the following code to a plugin/mu-plugin. It will trigger the issue:
function test_create_product_objects() {
	$product1 = new WC_Product_Simple();
	$product2 = new WC_Product_Simple();
}

add_action( 'admin_init', 'test_create_product_objects' );
  1. Go to Products > Add New
  2. Observe the tip in the bottom of the product image metabox.
  3. Click the new link and ensure it works as expected.
  4. Go to Posts > Add New
  5. Verify that the tip does not appear in the bottom of the featured image metabox.

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?
  • Have you created a changelog file for each project being changed, ie pnpm changelog add --filter=<project>?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@mattsherman mattsherman added Bug The issue is a confirmed bug. Product/Inventory Management Issues related to product or product page. labels Sep 20, 2022
@mattsherman mattsherman self-assigned this Sep 20, 2022
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Sep 20, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 20, 2022

Test Results Summary

Commit SHA: c44157a

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests11800201200m 44s
E2E Tests185002018713m 56s
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.

@SpottedPaint
Copy link

Would it be better to make the max file size the PHP upload size limit. I wouldn't have thought that 2GB was a realistic upload size for most hosting. Unless thats a limit of something else. The intention of this message is presumably originally to be helpful to a more non technical user.
In the wp-admin/includes/media.php file it does
printf( __( 'Maximum upload file size: %s.' ), esc_html( size_format( $max_upload_size ) ) );
the $max_upload_size comes from a function wp_max_upload_size();
So that may be a WP way of doing it.

@LucianHus
Copy link

LucianHus commented Sep 20, 2022

@mattsherman - I have found the cause: disabling All in One SEO Pro plugin will eliminate this error (at least in case of the shops managed by me).

@mattsherman
Copy link
Contributor Author

@SpottedPaint

Would it be better to make the max file size the PHP upload size limit.

Totally. Thanks for pointing this out!

I wouldn't have thought that 2GB was a realistic upload size for most hosting.

Agreed. It is not!

Copy link
Contributor

@louwie17 louwie17 left a comment

Choose a reason for hiding this comment

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

Thanks for jumping on this so quickly @mattsherman, and nice straight forward fix, this tested well.

@mattsherman mattsherman merged commit 9d96419 into trunk Sep 20, 2022
@mattsherman mattsherman deleted the fix/multiple-product-image-upload-tips branch September 20, 2022 12:09
@github-actions github-actions bot added this to the 7.1.0 milestone Sep 20, 2022
@github-actions
Copy link
Contributor

Hi @mattsherman, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

github-actions bot pushed a commit that referenced this pull request Sep 20, 2022
…pages (#34739)

* Only show the product image upload tip once, and on all product edit pages.
* Use max upload size in message.
roykho pushed a commit that referenced this pull request Sep 20, 2022
…pages (#34739)

* Only show the product image upload tip once, and on all product edit pages.
* Use max upload size in message.
roykho pushed a commit that referenced this pull request Sep 21, 2022
…pages (#34739)

* Only show the product image upload tip once, and on all product edit pages.
* Use max upload size in message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug The issue is a confirmed bug. plugin: woocommerce Issues related to the WooCommerce Core plugin. Product/Inventory Management Issues related to product or product page.

Projects

None yet

5 participants