Gift Certificate module re-issues multiple certificates when order status is changed.

Hmm, definitely a problem. First off, I'm not sure why the module allows the admin to select more than one status for when certs are issued. That just seems to be asking for trouble without giving any real benefit. Anyone with objections if we make it a single-select?

Second, the problem of re-issuing certs if an admin changes the status back and forth is a "problem" inherent in ubercart. I've seen this problem in other modules, like ubercart_userpoints, where if you go to the completed status multiple times the action happens multiple times. But on the other hand, I could see there could be some cases where you want it re-issued, such as if there was a problem with the order and so the admin expects it to be issued again when it goes to complete. So, I'm not sure if there is a good solution to it being issued multiple times if the admin is changing the status back and forth.

No issues here (with the single-select). Make it so :)

Sleepingmonk is right -- there are use cases where triggering the gift certificate create/update functions on a single status would be too limiting. In fact, I'm running into the issue on a current project.

I've got two different payment gateways set up -- Authorize.net and PayPal WPS. With Authorize.net, I'm authorizing but not capturing funds right away. With PayPal, I'm capturing right away -- except that PayPal doesn't capture immediately when a customer pays by check.

Finally, when a user makes a purchase with a gift certificate and uses the Zero Total payment method, the status is just Pending.

In other words, I've got the potential for orders going into any of three states after a purchase: Pending, PayPal Pending, Payment Received (which automatically gets reset to Completed by conditional actions).

So I do need to be able to check off multiple states in the Gift Certificate settings. But I obviously don't want the gift certificate amount being added (on create) or subtracted (on update) multiple times as an order moves through the various states during fulfillment.

Here's my proposed solution:

On the create side (when a cert is purchased): Fortunately, the uc_gift_certificate.module uses the product data field in the order object to store the certificate code when a new certificate is created. So we can check if that's already set when examining the order in the uc_gift_certificate_create_new_certs() function. If it's set, then we don't try to create a new certificate.

if ($data && $data['attributes'] && isset($data['attributes']["Recipient's Email Address"][0]) && !isset($data['attributes']['Certificate Code'][0])){
  ...
}

On the update side (when a cert is used for a purchase): We can use the data field of the line item to set a flag that the user's certificates have already been deducted the appropriate amount. We test for this flag before making deductions so that we don't over-deduct from someone's certificates.

if(!isset($li_data['uc_gift_certificate']['cert_updated']) || !$li_data['uc_gift_certificate']['cert_updated']) {
 ...
      $li_data = is_array($li_data) ? $li_data : array();
      $li_data['uc_gift_certificate'] = array('cert_updated' => true);
      uc_order_update_line_item($li->line_item_id, $li->title, $li->amount, $li_data); //store new data back into record
}

Your thoughts? This seems to work in my initial tests, but I'd like to know from those of you who are more familiar with the module if there are any gotchas to look out for.

<?php

I am having this same problem with duplicate gift certificates being sent out because of the nature of the order status update workflow.

@aasarava

I'm very interested in testing your code, I was able to find the location in uc_gift_certificate.module where your first block of code was intended to be modified. But I'm not sure I know where the second block was intended to be inserted. I see 5 spots in the code where uc_order_update_line_item is called. I'd like to help you test your solution out with the hopes of seeing if it could make it into the module, but I'd like to know with a little more certainty where the second block of code is to go so I'm not testing something different than you did. Can you provide a diff or even a hint or two to help out?

Thanks,
Jeremy Zerr
http://www.zerrtech.com

Jeremy,

The second block goes in uc_gift_certificate_update_cert_vals(). I'd send a diff, but I've gone on and done a huge rewrite to the module, so I'm afraid it wouldn't be so helpful. I may, if I can get some free time one of these days, wrap up the entire thing and post it here in case all my changes are helpful to the maintainers. Unfortunately, I don't have the time at the moment to break out the individual changes and upload them as patches.

Good luck!

If you want to post your module (or a link we can get to it) I'd love to take a look. And if you can comment with it the changes you've made, I think the module is about due for a bit of a rewrite in places.

Patch attached. It basically just rolls in aasarava's changes from above, with a bit of variable name changing since it wasn't totally drop-in. Seems to work, it fixes multiple cert issuance if you purchase a cert with a cert (I know, who would do that?). I haven't finished testing, but it seems that it should take care of the issues that occur when the order status gets changed from one issuing/deducting status to another.

Update: Tested it extensively with Paypal Website Payments Standard and Website Payments Pro. Didn't run into any duplicate issuance, and the issuance on multiple order statuses seemed to work well, since Paypal WPS checkouts end up on a different order status than WPP checkouts did.

Setting to needs review as there is a patch here.

Can you re-roll the patch against HEAD, some field names have changed (specifically value -> remaining_value).

Used the patch from the first post, from sleepingmonk, and worked fine (did not multiply the gift amount because the order status change), BUT email notificatión was not send to the recipient...

Any ideas?