Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
82 stars 29 forks source link

Update Subscriptions Data Copier to be more robust and avoid assumptions about setters #419

Open james-allan opened 1 year ago

james-allan commented 1 year ago

Description

The WC_Subscriptions_Data_Copier class currently makes assumptions about setters. If a piece of custom metadata exists with a meta key that maps to a WC_Order setter. eg _shipping_address -> set_shipping_address() it uses that setter without any guarantee that setter was designed to accept or set that kind of data.

We should tighten the data copier up so it only calls the core set of order setters for know data keys.

To generate a list of known setters, we can use get_order_data(), get_address_data() and get_operational_data() functions.

Those functions currently return the core order data via getters, this will be a good reference for what core data is and what setters exist.

Testing instructions

  1. Purchase a subscription.
  2. Simulate a custom plugin adding custom meta that maps to a order setter. (see example code snippet below).
  3. Go to WooCommerce > Subscriptions
  4. Edit the subscription.
  5. From the actions dropdown select "Process renewal"
  6. Observe error.
$subscription = wcs_get_subscription( 123 );
$subscription->update_meta_data( '_shipping_address', 'dummy_data' );
$subscription->save()

Since https://github.com/Automattic/woocommerce-subscriptions-core/pull/417/, these steps to replicate won't actually lead to a fatal error, but this issue illustrates the broader issue with the data copier making assumptions about a setter purely because it exists. We should be more defensive and put guardrails around the data copier so it doesn't do things we don't expect or anticipate.