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

Deprecated creation of dynamic properties #38857

Closed
4 of 5 tasks
Chouby opened this issue Jun 21, 2023 · 17 comments · Fixed by #44896
Closed
4 of 5 tasks

Deprecated creation of dynamic properties #38857

Chouby opened this issue Jun 21, 2023 · 17 comments · Fixed by #44896
Labels
focus: order Issues related to orders. focus: unit tests Related to PHP unit testing. priority: normal The issue/PR is of normal priority—not many people are affected or there’s a workaround, etc. team: Proton type: community contribution

Comments

@Chouby
Copy link
Contributor

Chouby commented Jun 21, 2023

Prerequisites

  • I have carried out troubleshooting steps and I believe I have found a bug.
  • I have searched for similar bugs in both open and closed issues and cannot find a duplicate.

Describe the bug

Running PHPUnit tests with WC 7.9 beta 1 + PHP 8.2, deprecated notices are logged:

PHP Deprecated:  Creation of dynamic property WC_Order_Item_Product::$legacy_values is deprecated in /home/travis/build/polylang/polylang-wc/tmp/woocommerce/plugins/woocommerce/includes/class-wc-checkout.php on line 517
PHP Deprecated:  Creation of dynamic property WC_Order_Item_Product::$legacy_cart_item_key is deprecated in /home/travis/build/polylang/polylang-wc/tmp/woocommerce/plugins/woocommerce/includes/class-wc-checkout.php on line 518

Expected behavior

No deprecated notice

Actual behavior

N/A

Steps to reproduce

N/A (Im' running private PHPUnit tesfs)

WordPress Environment

WP 6.2
WC 7.9 beta 1
PHP 8.2

Isolating the problem

  • I have deactivated other plugins and confirmed this bug occurs when only WooCommerce plugin is active.
  • This bug happens with a default WordPress theme active, or Storefront.
  • I can reproduce this bug consistently using the steps above.
@github-actions github-actions bot added status: awaiting triage This is a newly created issue waiting for triage. type: community contribution labels Jun 21, 2023
@samueljseay samueljseay added focus: order Issues related to orders. status: reproduction Bug reports that need to be reproduced and confirmed. and removed status: awaiting triage This is a newly created issue waiting for triage. labels Jun 22, 2023
@github-actions
Copy link
Contributor

We are adding the status: needs reproduction label to this issue to try reproduce it on the current released version of WooCommerce.

Thank you for your patience.

@Chouby
Copy link
Contributor Author

Chouby commented Jul 31, 2023

@samueljseay I understand that most often issues require reproduction. However this is not the case hare. It's just about two properties missing declaration. A static analysis of the code is sufficient to aknowledge and fix the issue.

@samueljseay
Copy link
Contributor

@Chouby absolutely, this is more of a triaging process thing from our perspective when we receive community contributed issues. Confirmation the bug or issue still exists so we can prioritize it may indeed involve just running static analysis tools to confirm the problem :)

@rrennick
Copy link
Contributor

rrennick commented Aug 3, 2023

@Chouby I ran the unit tests with trunk and am not seeing any PHP deprecation warnings so it looks like this has been addressed in other PRs. Could you run the tests and confirm?

@rrennick rrennick added needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. team: Proton focus: unit tests Related to PHP unit testing. labels Aug 3, 2023
@pogla
Copy link

pogla commented Oct 12, 2023

@samueljseay
What's the ETA for resolving this? We're seeing the same notices after updating to PHP 8.2.

@samueljseay
Copy link
Contributor

@pogla I was just triaging this issue to help the teams responsible for working on it.

cc @rrennick can you repro with PHP8.2?

@rrennick
Copy link
Contributor

@samueljseay I re-read my comment from Aug. I should have included that I did test with PHP 8.2.

We're seeing the same notices after updating to PHP 8.2.

Are you seeing these running unit tests?

Copy link
Contributor

As a part of this repository's maintenance, this issue is being marked as stale due to inactivity. Please feel free to comment on it in case we missed something.

After 7 days with no activity this issue will be automatically be closed.

@github-actions github-actions bot added the status: stale Issues that have no had any activity for some time. label Nov 17, 2023
@pogla
Copy link

pogla commented Nov 17, 2023

I'm pretty sure this issue should not me marked as stale.

@rrennick rrennick removed the status: stale Issues that have no had any activity for some time. label Nov 17, 2023
@rrennick
Copy link
Contributor

@pogla Can you provide the steps needed to reproduce the issue? Here's the log from running the tests today with the section listing the skipped tests removed:

# vendor/bin/phpunit                      
Installing...
Running as single site... To run multisite, use -c tests/phpunit/multisite.xml
Installing WooCommerce...
Not running ajax tests. To execute these, use --group ajax.
Not running ms-files tests. To execute these, use --group ms-files.
Not running external-http tests. To execute these, use --group external-http.
PHPUnit 9.6.11 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.12
Configuration: /Users/ronrennick/Sites/wc/wp-content/woocommerce/plugins/woocommerce/phpunit.xml

.............................................................   61 / 3671 (  1%)
.............................................................  122 / 3671 (  3%)
.............................................................  183 / 3671 (  4%)
.............................................................  244 / 3671 (  6%)
.............................................................  305 / 3671 (  8%)
.............................................................  366 / 3671 (  9%)
.............................................................  427 / 3671 ( 11%)
.............................................................  488 / 3671 ( 13%)
.............................................................  549 / 3671 ( 14%)
.............................................................  610 / 3671 ( 16%)
.............................................................  671 / 3671 ( 18%)
.............................................................  732 / 3671 ( 19%)
.............................................................  793 / 3671 ( 21%)
.............................................................  854 / 3671 ( 23%)
.............................................................  915 / 3671 ( 24%)
.............................................................  976 / 3671 ( 26%)
............SSSSSSSS......................................... 1037 / 3671 ( 28%)
............................................................. 1098 / 3671 ( 29%)
............................................................. 1159 / 3671 ( 31%)
............................................................. 1220 / 3671 ( 33%)
............................................................. 1281 / 3671 ( 34%)
............................................................. 1342 / 3671 ( 36%)
............................................................. 1403 / 3671 ( 38%)
............................................................. 1464 / 3671 ( 39%)
............................................................. 1525 / 3671 ( 41%)
............................................................. 1586 / 3671 ( 43%)
............................................................. 1647 / 3671 ( 44%)
............................S................................ 1708 / 3671 ( 46%)
............................................................. 1769 / 3671 ( 48%)
............................................................. 1830 / 3671 ( 49%)
............................................................. 1891 / 3671 ( 51%)
............................................................. 1952 / 3671 ( 53%)
.......................................S............You have attempted to register a duplicate item with WooCommerce Navigation: `test-duplicate-item`
......... 2013 / 3671 ( 54%)
............................................................. 2074 / 3671 ( 56%)
............................................................. 2135 / 3671 ( 58%)
............................................................. 2196 / 3671 ( 59%)
............................................................. 2257 / 3671 ( 61%)
..................................S.......................... 2318 / 3671 ( 63%)
............................................................. 2379 / 3671 ( 64%)
............................................................. 2440 / 3671 ( 66%)
............................................................. 2501 / 3671 ( 68%)
............................................................. 2562 / 3671 ( 69%)
............................................................. 2623 / 3671 ( 71%)
.................................S........................... 2684 / 3671 ( 73%)
........................S.................................... 2745 / 3671 ( 74%)
............................................................. 2806 / 3671 ( 76%)
............................................................. 2867 / 3671 ( 78%)
............................................................. 2928 / 3671 ( 79%)
............................................................. 2989 / 3671 ( 81%)
........................S.................................... 3050 / 3671 ( 83%)
............................................................. 3111 / 3671 ( 84%)
............................................................. 3172 / 3671 ( 86%)
............................................................. 3233 / 3671 ( 88%)
............................................................. 3294 / 3671 ( 89%)
............................................................. 3355 / 3671 ( 91%)
............................................................. 3416 / 3671 ( 93%)
............................................................. 3477 / 3671 ( 94%)
............................................................. 3538 / 3671 ( 96%)
............................................................. 3599 / 3671 ( 98%)
............................................................. 3660 / 3671 ( 99%)
...........                                                   3671 / 3671 (100%)

Time: 07:07.725, Memory: 233.00 MB

There were 14 skipped tests:
<snip>
OK, but incomplete, skipped, or risky tests!
Tests: 3671, Assertions: 13020, Skipped: 14.

Copy link
Contributor

As a part of this repository's maintenance, this issue is being marked as stale due to inactivity. Please feel free to comment on it in case we missed something.

After 7 days with no activity this issue will be automatically be closed.

@github-actions github-actions bot added the status: stale Issues that have no had any activity for some time. label Nov 25, 2023
@rrennick
Copy link
Contributor

The function call here is WC_Checkout::create_order_line_items().

@rrennick rrennick removed needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. status: stale Issues that have no had any activity for some time. labels Nov 27, 2023
@coreymckrill coreymckrill added priority: normal The issue/PR is of normal priority—not many people are affected or there’s a workaround, etc. and removed status: reproduction Bug reports that need to be reproduced and confirmed. labels Dec 14, 2023
@mikkamp
Copy link
Contributor

mikkamp commented Dec 18, 2023

@rrennick Just following up on this issue as I'm able to reproduce with the latest version of trunk as well. It was initially discovered while running AutomateWoo unit tests.

I'm actually getting 4 deprecation notices:

[18-Dec-2023 15:01:30 UTC] PHP Deprecated:  Creation of dynamic property WC_Coupon::$sort is deprecated in woocommerce/includes/class-wc-cart-totals.php on line 375
[18-Dec-2023 15:01:30 UTC] PHP Deprecated:  Creation of dynamic property WC_Order_Item_Product::$legacy_values is deprecated in woocommerce/includes/class-wc-checkout.php on line 520
[18-Dec-2023 15:01:30 UTC] PHP Deprecated:  Creation of dynamic property WC_Order_Item_Product::$legacy_cart_item_key is deprecated in woocommerce/includes/class-wc-checkout.php on line 521
[18-Dec-2023 15:01:30 UTC] PHP Deprecated:  Creation of dynamic property WC_Order_Item_Shipping::$legacy_package_key is deprecated in woocommerce/includes/class-wc-checkout.php on line 607

Steps to reproduce:

  1. Set up a site on PHP 8.2 with WP_DEBUG logging enabled
  2. Setup the store with some basic products
  3. Create a simple coupon
  4. Create a simple shipping rate (all countries)
  5. Use a block based checkout page
  6. Add a product, add a coupon
  7. View the PHP log (or on screen / in API requests if configured to output notices) to see deprecation notices
  8. Can be reproduced with shortcode cart / checkout page, but we must complete an order to see the notices

Running the unit tests included with WooCommerce is not sufficient because it explicitly turns off deprecation notices here: https://github.com/woocommerce/woocommerce/blob/8.4.0/plugins/woocommerce/tests/legacy/bootstrap.php#L98
If I disable that line, I can run the following 3 unit tests which identify these notices:

vendor/bin/phpunit --filter=WC_Order_Functions_Test
vendor/bin/phpunit --filter=WC_Cart_Totals_Tests
vendor/bin/phpunit --filter=WC_Tests_Orders

If I run all the tests I get a lot more deprecation notices, but these are the ones I identified which can occur during a regular checkout and depending on configuration could block a checkout from completing.

@jdevieux
Copy link

jdevieux commented Jan 3, 2024

👋🏾 @mikkamp Is this issue still being worked on, or was it resolved within [#1657]? (https://github.com/woocommerce/automatewoo/pull/1657)

One of our third-party developers is still experiencing this issue:

When using WooCommerce with php8.2, we are not able to place an order when an WooCommerce coupon is applied to the cart. We are getting the following error,
"Deprecated: Creation of dynamic property WC_Coupon::$sort is deprecated in C:\wamp64\www\test\wp-content\plugins\woocommerce\includes\class-wc-cart-totals.php on line 378"
Please note that this issue doesn't happen when using WooCommerce Blocks but only during WooCommerce Checkout flow.

@mikkamp
Copy link
Contributor

mikkamp commented Jan 4, 2024

The issue https://github.com/woocommerce/automatewoo/pull/1657 only addresses making the AutomateWoo code compatible with PHP 8.3. The notices in WooCommerce core still need to be addressed in this issue.

@wavvves
Copy link
Contributor

wavvves commented Feb 1, 2024

"Deprecated: Creation of dynamic property WC_Coupon::$sort is deprecated in C:\wamp64\www\test\wp-content\plugins\woocommerce\includes\class-wc-cart-totals.php on line 378"

@jdevieux for the record, that specific error going to be fixed in #43872 if it ends up getting reviewed and merged as it got in the way of that PR. You might want to keep an eye on the PR's progress ;)

@wavvves
Copy link
Contributor

wavvves commented Feb 16, 2024

for the record, also reported in #44621

Konamiman added a commit that referenced this issue Feb 28, 2024
The `\WC_Checkout::create_order_line_items` method assigns values to properties of the `WC_Order_Item_Product` class which did not exist.

PHP Deprecated:  Creation of dynamic property WC_Order_Item_Product::$legacy_values

PHP Deprecated:  Creation of dynamic property WC_Order_Item_Product::$legacy_cart_item_key

Fixes #38857

---------

Co-authored-by: Néstor Soriano <konamiman@konamiman.com>
Konamiman added a commit that referenced this issue Mar 13, 2024
The `\WC_Checkout::create_order_line_items` method assigns values to properties of the `WC_Order_Item_Product` class which did not exist.

PHP Deprecated:  Creation of dynamic property WC_Order_Item_Product::$legacy_values

PHP Deprecated:  Creation of dynamic property WC_Order_Item_Product::$legacy_cart_item_key

Fixes #38857

---------

Co-authored-by: Néstor Soriano <konamiman@konamiman.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: order Issues related to orders. focus: unit tests Related to PHP unit testing. priority: normal The issue/PR is of normal priority—not many people are affected or there’s a workaround, etc. team: Proton type: community contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants