My client needs some additional data visible in their DPS account for each payment, which they would like appended to the merchant reference #. As far as I can see, the only way to do this is by adding a drupal_alter() from commerce_dps_pxpay_order_form(). I've tested this as follows without incident:

/**
 * Called from commerce_dps_pxpay_redirect_form().
 */
function commerce_dps_pxpay_order_form($form, &$form_state, $order, $settings) {
  module_load_include('inc', 'commerce_dps_pxpay', 'commerce_dps_pxpay');

  $transaction = array(
    'user_id' => $settings['commerce_dps_pxpay_userid'],
    'server' => $settings['commerce_dps_pxpay_server'],
    'key' => $settings['commerce_dps_pxpay_key'],
    'amount' => $order->commerce_order_total['und'][0]['amount'] / 100,
    'type' => 'Purchase',
    'txn_id' => uniqid($order->order_number),
    'reference' => $settings['commerce_dps_pxpay_refprefix'] . " #" . $order->order_number,
    'currency' => 'NZD',
    'url_success' => $settings['return'],
    'url_failure' => $settings['cancel_return'],
    'email' => $order->mail,
    'log' => $settings['commerce_dps_pxpay_log_transactions'],
  );
  
  // Allow other modules to alter the transaction data before sending it to DPS.
  drupal_alter('commerce_dps_pxpay_order_form_transaction', $transaction, $order);

  if ($url = commerce_dps_pxpay_generate_request($transaction)) {
    drupal_goto($url);
  }
  else {
    watchdog('commerce_dps_pxpay', 'Unable to generate DPS request with settings: @settings', array('@settings' => print_r($settings, 1)), WATCHDOG_DEBUG);
  }
}

This allows altering the entire transaction, but perhaps this could be considered too open to casual abuse? In which case maybe a limited subset of the transaction array could be passed to drupal_alter() and merged with the full transaction array post-alter.

Comments

johnpitcairn’s picture

Status: Needs work » Needs review
StatusFileSize
new600 bytes

Patch attached, code as above. DO NOT USE!

johnpitcairn’s picture

Status: Active » Needs review

Erk. Ignore the above - it won't validate on return from DPS because the DPS transaction ref no longer matches what commerce_dps thinks it should be. I need to drupal_alter() into all code that generates the transaction ref. But it's Friday and I'm knocking off.

johnpitcairn’s picture

Status: Needs review » Needs work

>> needs work

johnpitcairn’s picture

OK, I couldn't let it go - here's plan B.

Merchant reference generation is wrapped in a new function, commerce_dps_merchant_reference(). This includes a drupal_alter, which can be implemented as hook_commerce_dps_merchant_reference_alter(&$reference, $order, $settings). Two lines in the existing module code have been updated to call commerce_dps_merchant_reference(), instead of hard-coding the reference-generation logic in two separate places.

This seems to work for me, and should be a lot safer than allowing a full $transaction alter.

xurizaemon’s picture

That seems nice.

As an alternative approach, if we swapped the existing merchant reference prefix with a tokenised merchant reference string, would that have solve your issue equally well?

Or is the extra data you need not available through order (and related, eg user) tokens?

johnpitcairn’s picture

I thought about that, but the data I need to expose is derived from the grandparent entity of a commerce product in the order - I think that might be quite a lot more difficult to implement at token_replace() time compared to the drupal_alter(). But I agree token support in the merchant reference field would be a good thing for site builders.

johnpitcairn’s picture

Status: Needs review » Reviewed & tested by the community

Can I RTBC this myself? It's been running fine for a few months now.

I think token support in the UI should be a separate issue, but it's not something I need right now.

xurizaemon’s picture

It's naughty to self-RTBC, but so is being a slack maintainer. This looks pretty innocuous to me, you've packaged it up nicely.

I'll give it a spin and see. If you want to supply an example of how this might be used in an interesting way, we can add it to the docs.

johnpitcairn’s picture

Well in my case the commerce product is never created directly by the web editor, it is automatically generated and referenced by a field collection instance, which is itself attached to a parent node, and that references another node. And the client wanted the ID of that node as part of the reference in the DPS transaction, so they can track that when it turns up in their bank account. So it's useful if you need to add something that is generated completely independently from any commerce module.

johnpitcairn’s picture

Any chance of a commit for this?

johnpitcairn’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new2 KB

Sigh. Patch no longer applies, re-rolled. The hook is now commerce_dps_pxpay_merchant_reference_alter(), for consistency with the rest of the module.

xurizaemon’s picture

Assigned: johnpitcairn » Unassigned
Issue summary: View changes
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.