POC: Transaction Objects

[ChaseWiseman]

POC only, DO NOT MERGE

Objective

Provide a new way for gateways to pass transaction data around throughout the payment, capture, and refund processes that's more reliable and testable, and cut down the size of our gateway classes.

Problem

Currently, transaction data is added directly to WC_Order objects by assigning new properties that often become their own stdClass objects which hold various bits of data. This can be credit card information or previous transaction details in the case of refunds. Historically, this has been a fine way to make important data available before and after calling a gateway's API. However, this may not to be so reliable in the future as we've seen with WooCommerce 3.0 compatibility and the move away from directly accessing WC_Order properties.

Solution

This PR introduces transaction objects. The purpose here is to decouple our gateway-specific data from WC_Order and have a consistent set of objects that are passed around instead, with the order merely being one of many properties associated with a single transaction. In this PR you will see a few familiar transaction types:

• Payment
  • Authorization
  • Charge
• Capture
• Refund
• Void

All of these types would have a set of common properties accessible via getters and setters, which gateways would set as they become available, like transaction_id, and get as needed when adding order notes, etc... Right now these are mostly based on the meta we currently save to orders, as well as what's available via the Shopify API

Payment Methods

Another object added here is payment method. These would live inside the transaction object for any transaction type that requires payment information. Like transactions themselves, these would have getters and setters specific to their type, like set_csc() or set_check_number(). One of the biggest benefits here is cleaning up some of the logic within the gateway classes surrounding credit cards vs. eChecks, keep the transaction objects themselves clear of specific payment types, as well as autofilling a lot of these properties if a token is present.

Implementation

Within process_payment(), we could simply call a method such as $transaction = $this->create_payment_transaction(), which would generate a new transaction object, which would be overrideable by the concrete gateway in case they need their own properties. We'd then go about setting various properties like amount, currency, etc...

In the case of direct payment transactions, we could similarly generate a payment method object and set it's properties like so:

protected function set_posted_credit_card_data( SV_WC_Payment_Gateway_Transaction_Credit_Card $payment_method ) {

	$card_number = SV_WC_Helper::get_post( 'wc-' . $this->get_id_dasherized() . '-account-number' );
	$card_type   = SV_WC_Helper::get_post( 'wc-' . $this->get_id_dasherized() . '-card-type' );
	$exp_month   = SV_WC_Helper::get_post( 'wc-' . $this->get_id_dasherized() . '-exp-month' );
	$exp_year    = SV_WC_Helper::get_post( 'wc-' . $this->get_id_dasherized() . '-exp-year' );
	$csc         = SV_WC_Helper::get_post( 'wc-' . $this->get_id_dasherized() . '-csc' );

	// add card type for gateways that don't require it displayed at checkout
	if ( empty( $card_type ) ) {
		$card_type = SV_WC_Payment_Gateway_Helper::card_type_from_account_number( $card_number );
	}

	// handle single expiry field formatted like "MM / YY" or "MM / YYYY"
	if ( $expiry = SV_WC_Helper::get_post( 'wc-' . $this->get_id_dasherized() . '-expiry' ) ) {
		list( $exp_month, $exp_year ) = array_map( 'trim', explode( '/', $expiry ) );
	}

	$payment_method->set_card_number( $card_number );
	$payment_method->set_card_type( $card_type );
	$payment_method->set_exp_month( $exp_month );
	$payment_method->set_exp_year( $exp_year );
	$payment_method->set_csc( $csc );

	return $payment_method;
}

Then add that to the trasaction object: $transaction->set_payment_method( $payment_method );

All of this fantastic data would then be available down the line as we make API calls:

$api_data = array(
    'description' => $transaction->get_description(),
    'cc_number'   => $transaction->get_payment_method()->get_card_number(),
    // etc...
);

The Future

For now these would live and die only during the payment/capture/refund/void process, but eventually they could handle storing data via a save() method. This could be order meta as we do now, or if we wanted to store each transaction in a custom table down the line. With that we could extend the order admin with a more detailed list of transaction events, or make each transaction available via the REST API like Shopify.

Eventually this could be a candidate for a core contribution if dealing with transactions in this manner proves useful.

What do y'all think? Any glaring problems or omissions you see here? I would love some feedback before going any further with actual gateway implementation.

[bekarice]

cc @skyverge/wc-code-review

[unfulvio]

from a coding / architecture standpoint I am definitely in favor of adopting getter/setters for the reasons explained above

the concern when dealing with payment data would be of course security but I can't tell if there are any pitfalls with the proposed approach, for instance in the way CC attributes are stored and passed between objects

[justinstern]

outstanding PR writeup 🍻

code is looking good, I'd say that this concept of extracting out transactions is a great future contribution to core

[maxrice]

agreed, really great PR writeup and code looks very solid. As a next step, I think it's worth implementing this in the framework gateway classes so we can see how it would all fit together.

As part of that to keep things really flexible, try to make it such that an implementing gateway can a) extend the transaction class and b) return an instance of that extended class inside the FW code. This would be similar to how we did the payment form stuff.

I also agree this would make an ideal core contribution at some point, especially with how it extends WC_Data.

@unfulvio:

the concern when dealing with payment data would be of course security but I can't tell if there are any pitfalls with the proposed approach, for instance in the way CC attributes are stored and passed between objects

A fair point -- my thinking in the past with this sort of thing is that usually when raw CC data is being passed between objects, it's already available via $_POST anyhow, so there's nothing particularly "risky" about setting the data on existing objects in memory. This is somewhat less of a concern these days with a lot of direct gateways moving to some sort of client-side JavaScript tokenization.