Gift Certificates are not created (when purchased) if order goes right to Completed

Any progress? Need any help? We've deployed this for a client, and it's not working -- I think for exactly these reasons.

We've done a ton of patching of this module, and made a lot of fixes. Not sure I can easily extract a patch for this issue alone.

Basically, here's what we've done so far:

1. Add a "chart" table to act as a log, and track any time a gc value is changed -- by admin, purchase, redeem, or import
2. When creating a gift certificate, check to see if we've already created one associated with this order and user. If we have, don't do it again. This allows you to set multiple order states for purchase of a gift certificate, and assures no multiple generation.
3. Patch javascript to hide the "free order" payment method if there's a balance, and hide the other payment methods if the gift-certificate amount makes the total 0.
4. Recalculate the order balance and gift certificate balance whenever a line item changes (this is still happening way too much, though!)
5. Because gift certificate balance is now added as a line item at checkout, and payment is in full, conditional actions fire and order status now gets properly updated to payment received, and the gift card balance can properly get redeemed.
6. Created a gift certificate user tab to view what certificates are assigned to a user, and a "log" screen to show balance changes for each certificate.
7. Changed the admin create screen to allow admins to create certificates for an email address, and skip having to create an account first (make it behave the same way the normal order process goes)

We've also ripped out some of the coupon functionality, since uc_coupon already handles that.

So what we've got has changed substantially from the core uc_gift_certificates. Would be happy to contribute this back... should I just attach a patch here, upload a modified version of this module, or create a new issue?

Cheers,
John

I vote for the 'upload a modified version' option for testing at least.

I second jday's vote. @freelock if you are still willing to contribute.

Sounds like @freelock's contribution would give this module a great boost. Can't wait.

Hi,

Been on vacation, back now...

Okay, I've got a lot more work to do on this, but as is it's a pretty good improvement. I'm still planning to improve the admin ordering process -- at the moment, gift certificate amounts recalculate on the order screen completely incorrectly. I believe the right fix is to implement the "total" operation in the hook_order, and remove the calculation from the "save" operation, where it does some screwball things.

There's also a lot of other code in there I do not believe is being used, so a good refactoring to remove cruft is in order...

What does work is the front-end checkout process, certificate balance history, admin assigning balances to an email address and have it either associate with the correct account or create new account/send credentials.

Cheers,
John

This is probably the wrong issue to provide these comments, but we've got a short list of other things to do:

1. Revamp hook_order to calculate a correct value on the admin order screen
2. Add a CA trigger for gift certificate creation, that passes in the order object
3. Add a CA action for adjusting the gift certificate balance

The last two are a client request--they want to allow people to purchase a $500 certificate and get an extra $25, or purchase a $1000 certificate and get an extra $100...

And finally, we'd like to go remove all the (apparently) legacy code that makes this module difficult to troubleshoot/figure out. Not sure if we'll get to this soon -- we need to renegotiate budget with our client who needs this, and we have some unrelated work coming in we're going to need to focus on...

Hi, torgosPizza,

I'm seeing a lot of code related to coupons in the module, and I've stripped away some of this. So if anything actually uses that code, my changes would break it -- however, we're not using that code and I haven't seen anything that actually calls the functions I've removed. So I'm guessing those are legacy functions from this project being based on uc_coupon perhaps? If that's the case, then my changes should be a drop-in replacement.

I did patch the uc_gift_certificate.install file, so it should upgrade the database adding the new table for logging gift certificate details as well as importing a line for each existing certificate.

Quite busy these days, and this project is on the backburner. But I can probably get you a diff from CVS sometime soon, and I'd be happy to help maintain the project...

frelock,
I have take your work and moved it a little further.
1. modified your tab in the user account so it displays both sent and recieved certificates for the user and the admin
2. added a new privilege level to allow users to see their own certificates
3. made a somewhat decent "view' page to look at a certicates, to support people purchasing "printable' gift certificates
4. i have put back the coupon stuff because i need to support it since people will buy certs and they need to be able to enter them on the checkout page.. interesting thing about that code was that the old javascript callbacks seem like they have never worked in 6.0 .. i have also made it a configurable option for the site, so if some doesn't want that functionality they can turn it off.

I looked at the code you have taken out its not much, but you stopped using a bunch of old code, and by the looks of it a lot of it wasn't being used anyway in the released version.

I am about ready to post my variation of your code, and if you were to post the diffs between what you have now vs what you posted it would be very helpful.

aniskhwaja, I have not done any further work on this module since I posted the code in #8.

Cheers,
John

torgosPizza,

As requested in #11, here's a patch containing a diff between 6.x-1.x-dev and the version I posted in #8.

Cheers,
John

John, Erik
One question, this my first 'big' drupal thing, one thing that is totally confusing me is uc_gift_certificate_cert_checkout function, its a fairly big function but I don't see anyone calling it. Is it being used at all?

Looks like it has the handler for the coupon form, but I don't see how it is invoked.

Thanks
anis

Hi,

My guess is that uc_gift_certificate_cert_checkout is a callback function to be used in uc_gift_certificate_checkout_pane, which somebody probably copied over from uc_coupon. At least that's my assumption.

You can see I added a debugging message using drupal_set_message, and must never have hit it (or I would've certainly removed it!) I'm sure this function can (and should) be entirely removed.

My assumption in this is that you could use the coupon block to get a gift certificate assigned to your account, but otherwise you would never enter a coupon code -- gift certificates are just associated with your account. Is there another use case we need to accommodate?

Cheers,
John

John,

I am actually beginning to wonder if that is the "real" handler for the check out form. And you keep reading these comments from Erik about how he uses the paper certificates, also on another thread where I pointed out some a security thread he proposed fixes in it.

So just for the heck of it, I changed the pane code to point to it and voila 80% of the issues with the module went away. As you know that the javascript shipped with both the dev and real version doesn't work at all, and you have cleaned it up quite a bit and your version version works. But when you turn on this handler the old javascript start making sense too.

But that function does things in very non-drupal way, and add buttons manually and "hooks" javascript code more like traditional non-cms code. So I wonder if Erik has turned it off and the new code is what he is working on and want to move forward.

Right now I am at cross-roads, I and probably a number of people need to support offline/paper coupon both using them and buying them. One course of action may be to integrate with Coupon and create the gift certificates as coupons if the user doesn't add an email address, the other course is building a parallel coupon system within this module. Although I have added a lot of coupon code in and was going to release the code today or tomorrow, but now I am beginning to wonder if it would not be a better option to create a coupon if there is no email address. Because as I am adding stuff in the checkout pane is becoming a real pain to keep straight. And its almost impossible to keep all the conditions right, especially with the Free module which keeps getting in the way.

The biggest problem in this code is that there is no separation of concerns and everything is mixed up together.

copied from http://drupal.org/node/617338#comment-3112098
so we keep the discussion on re-write in one place

freelock - June 21, 2010 - 05:37
Hi,

1. Not exactly sure when (if) the coupon code was actually being called. I did leave the redeem code in place (saw your other bug about this) as a mechanism for re-assigning certificate balances, but have not deployed this or tested.

2. Yes, with my version, having both checkboxes selected by default makes sense. Could definitely be further improved (I think a rewrite of uc_gift_certificate_order is highly called for, and may fix a lot of the remaining issues).

3. I continued the model of "if there's a gift certificate balance, use it first, and then apply other payment methods." If there's another use case that's desired, I would want to understand how it should work. As it is, the most substantial user interface change I made was fixing the Javascript to hide other payment methods if the gc balance covered the entire order, and hide the gc payment method if it doesn't. (Why force the user to make a choice here? Simpler is better!). And to achieve this, I made gift certificate balance appear on the checkout form, instead of on the next step, the review form (which was the source of a lot of problems!). Gift certificates interact with both the order line items and the payment methods -- and the logic is very twisted. Needs to be more separated.

5. Ah, looks like I'm a bit sloppy: drupal_add_css(drupal_get_path('module', 'uc_coupon') .'/reports.css', 'uc_coupon'); ... loading the css file from uc_coupon. Is that the error you're getting? We're running uc_coupon on this site as well as uc_gift_certificate.

Cheers,
John

Hi,

> The biggest problem in this code is that there is no separation of concerns and everything is mixed up together.

Totally agreed!

I still think the code that's working when you switch to that function is uc_coupon code that was copied into this to start the module.

I think the best thing to do is separate out the concerns:

- Gift certificate management, audit trails (mostly working in my code)
- Gift certificate purchase, with conditional actions support
- Applying gift certificate balance as line item (and providing option to not apply to a given order)
- Assigning a gift certificate to an account/paper certificates
- Setting payment method options based on final order total (working in my code)

If we can separate out the functions to do those basic tasks, I'm sure it will be a lot easier to figure out and maintain...

Cheers,
John

After evaluating integration with coupon module further, creating coupons with same coupon id as gift certificates is easy. So if they don't put an email address, what we simply do is not create the gift certificate at all, but add it in as a coupon which can be used for anything ( this can be expanded later in subsequent releases and we can perhaps even invoke panes from the coupon code to add restrictions )

What you have done in the checkout pane is clean, and we don't have to end up in this loopy situation where you need to add a coupon to the account and recalc it etc. And you let the coupon module do its thing which it does quite cleanly.

The only reason Coupon module doesn't support purchasing by a user directly, so will have to see how reporting will work.

There are however 3 issues that I am still looking for a solution for, so welcome the feedback:

1. Identifying the coupons bought by a user so they can reprint them
2. If a user wants to email a "gift certificate" that is really a coupon and they will expect normal GC behavior
3. Reporting since not all GCs will be GCs

The way I was thinking of handling this is:

1. Add a field in the gc table to indicate that its really a coupon not a GC and leave that sort of coupon out of GC calculations so its not double counted, but it can still be used in the summary page.

2. The email message and email behavior for the gift certificate that is really a coupon would be different, and they will be required to add it to their cart manually instead of adding it to their account like the normal gift certificate.

This way the module becomes very simple and focused, users get what they want printable and purchaseable gift certificates.

Because on a site no one wants to support two different places to enter gift certifaces and coupons, imagine how difficult it would be to communicate to customers, oh that is not a GC its a coupon put it in the coupon box.

Erik,
do you know what and where is is being called ?
/**
* Callback function for the discount line item.
*/
function uc_gift_certificate_line_item_discount($op, $order) {

Here is the code for hook_line_item, the documentation says that you have specify the callback in the hook, but the hook doesnt seem to specify a callback at all. Unless there is association that takes place because of the name, but the name here has the word discount in it, unless this is an undocumented or 6.0 hook for line_item_discount --

/**
 * Implementation of hook_line_item()
 */
function uc_gift_certificate_line_item() {
  $items[] = array(
    'id' => 'gift_certificate',
    'title' => t('Gift Certificates'),
    'weight' => 10,
    'stored' => TRUE,
    'default' => FALSE,
    'calculated' => TRUE,
    'add_list' => TRUE,
  );
  return $items;
}

Interesting what a little bit of googling surfaces, I wonder if this code started its life from here
http://drupal.org/node/694804 may be someone put it in as a placeholder and then didn't need it.

John/Erik,
One question for you guys, a lot of confusion in this module in the calculations is due to its dual nature, GC being discounts as well as a payment method. Also whenever a user has a GC there may still be balance that needs to be paid using another method PP or CC. So why have GC as a payment method at all?

Do guys see a problem if GCs were only discounts?

The Review given in this post is useful for us.I will implement all the facts and notes given here.Only to say thanks will not be adequate.I appreciate the writer for such wonderful and precious thoughts.
gift card for cash

Looks like we've picked up a spammer...

Anis,

The main point of the payment part of the module, as I understand it, is so you can check out if the gift certificate covers the entire balance of the order. Lots of payment modules won't check out with a zero balance -- or require you to enter a credit card number.

So removing the payment method would basically mean adding a dependency to some payment module that can handle a free checkout -- and then we'd still need to add code to make sure that's selected -- I think it's a much better user experience to have the payment methods automatically enable/disable/switch depending on the final balance...

The line item bit is confusing. IIRC, I tried adjusting its weight using this hook, but the actual code that populates its value is in the order hook. I would think adding a callback to calculate the actual balance of this line item is a much better place to put it--the trick being, to make sure that gets called/recalculated when the order balance changes. In our case, tax and shipping both get calculated through Ajax calls on the checkout page.

Also, the tax line items still appear later than the gift certificate line item--I thought I had set this to a high enough weight, but on our orders, you get a negative subtotal, and then an equal amount of tax that brings the final total back to 0. Correct behavior, but looks funny. But it's all totally incorrect if you do an admin order -- the line item amount actually subtracts its previous value from the total, and then calculates the difference. So every time you update the order in the admin screen, you get a different balance! The really strange thing is, the recalculation happens several times in a single request! It probably should only happen in the hook_order, when $op == 'total'.

Cheers,
John

John,
Thanks for the response. After struggling to understand this module, primarily because the intent isn't clear, and I am not sure If I gut it out and rebuild it, I would lose something that people are depending on, I have decided to stop work on this.

Talking about running balances, there is 2 very big issues in the module with anyone intending to use it for printable gift certificates.

1. when you buy or rather try to buy more than one anonymous certificate (i.e. no email addy no message) intending to give it two people you will get one certificate worth the combined total, so the user wil have to make multiple purchaces, which doesnt seem feasible.

2. The user balance is kept in form of individual certificates, so if you have multiple gift certificates and you make a purchase then the money comes out of multiple certificates. Every thing is fine so far, now if the transaction is reversed the whole balance will be returned to one certificate, which is fine too in case of email certificates. But in case of mailable/physical certificates that someone it trying to use anonymously it gets very confusing.

Also what we need is a module that allow purchasing one time use "coupons" rather than gift certificates, because we are not interested and keeping a running balance. So we have decided to build a system that lets user purchase coupons, and we let the Coupon module handle the redemption aspects.

But I have learnt an enormous amount reviewing this code, and thanks much to your and Erik's responses, I look forward to sharing the module with the community we are calling it Gift Coupon.

Regards
Anis

Anis,

Yes, I see this module as basically a "store credit" module, not really a gift certificate. I do think it would be useful to make redemption on any given order optional instead of automatic, and I would think that a paper certificate with a code to assign the balance to your account is also quite reasonable.

But it is different than a coupon, which uc_coupon (mostly) handles better. If it's not going to be a running balance, I would definitely agree that uc_coupon is the way to go. Be careful creating coupons for discounts of a single product--we got bit by a bug on that, which ended up applying the coupon value multiple times with multiple products in the checkout... suppose I need to go see if there's a bug already open for that!

Cheers,
John

I have added my module to ubercart.org, if there is interest, I might do more to it, for now it has everything I need for my goals.

http://www.ubercart.org/project/uc_gift_coupon

This module basically allows:
1. A site to to sell uc_copoun coupons
2. A site to give away coupons with product purchases.

It use product features and you can use almost all properties available in uc_coupons. This module will insert a coupon in the uc_coupons db.

It doesn't handle redemption uc_coupon does that.