When a sale is made to an anonymous user, no product keys are issued. Authenticated users get keys just as expected. The actual file downloads (handled by uc_file) get issued identically whether the user is anonymous or authenticated on checkout.

In my setup, conditional actions are configured so that as soon as payment is made, the order progresses to the Complete state. Likewise, if checkout completes with payment, the order will proceed to the Complete state. In which case, naturally one expects the keys to be issued.

This is using Paypal Website Payments Standard (WPS). I haven't tested with other payment systems.

I noticed the tiny paypal sub-module, but enabling it doesn't change the result. Looking at the code, though, I wouldn't expect it to. All it does is return true if the order is in the Payment Received state (this is apparently the intended test of when the order is "ready"). My configuration doesn't take any action for Payment Received and doesn't explicitly set that state anywhere. The order log does show a row in the order comments for Payment Received, but I'm not sure that's equivalent to actually entering that state.

Taking a quick look at the core module code, I would guess the issue arises in _uc_product_keys_order_ready() . There aren't any modules implementing "uc_product_keys_order_ready", so the foreach should pass through. If so, it means the default test is failing. Since it works based on a state transition, it might be the case that by the time this function fires, the order has already reached completion and will not undergo any further state transitions. In my judgment, a safer default test would be whether the order is in Payment Received or Completed, and has no keys currently issued. That should work even when no state transition ever occurs. This is all assuming that the _uc_product_keys_order_ready() actually executes, but it's called in multiple places and I don't see why it wouldn't.

The most puzzling part of all is why this differs in the authenticated case. I would figure that the same state transitions should occur regardless of whether the checkout user previously existed or is being created, but that doesn't seem to be so.

Version Details:

Drupal 6.16
Ubercart 2.2
uc_product_keys 1.0-rc2

Comments

freixas’s picture

freixas commented
Assigned: Unassigned » freixas

Hi,

Thanks for looking into this.

The first plan of attach would be for you to write a uc_product_keys_order_ready hook which implements whatever strategy you need to make the code work for your payment system. Once you have this tested and working, we can discuss whether this works as a generic solution or as a sub-module for PayPal Website Payments Standard or it's really specific to your system.

Testing whether an order has keys assigned is an expensive test. I'm not sure I would want to run the test any more than I have to.You might want to see how many times the test is run during the processing of an order both during checkout and during an admin edit.

By the way, be careful of the differences between order states and statuses. Right now, I can't remember which is which, but what the PayPal sub-module tests for is the opposite of the one I use. One is guaranteed to work on all installations and the other is payment system dependent (which is why it works well for hooks).

Now, as for the difference between anonymous and authenticated users, the only place you can assign product keys for anonymous users is when they are no longer anonymous. This occurs in hook_order_complete(), but not in hook_order(). The change from RC1 to RC2 was the addition of checking orders using both hooks.

I've come to the conclusion that there is no way to write the code correctly for all possible payment modules, which is why I also added the uc_product_keys_order_ready hook. Payment systems seem to like to change how they process the states (statuses?) and it's hard to rely on anything.

Let me know what you find out.

kagerato’s picture

kagerato commented

[Prior post contents deleted on 2010 April 13 ~12:00.]

Eureka. I've found a solution.

As I suspected in my initial post, _uc_product_keys_order_ready() is the reason the keys are not issued. The extraneous check for a state change will always fail in the paypal case. If you track the order states while performing a paypal checkout, you can see why.

There are two places where keys can be issued; the functions uc_product_keys_order() and uc_product_keys_uc_checkout_complete(), which are implementations of the ubercart hooks hook_order() and hook_uc_checkout_complete() respectively. The UC checkout complete function is the one meant to issue product keys for anonymous users.

Unfortunately, neither of these functions will fire in time to catch any state transition for an anonymous paypal checkout. In order for them to have an effect, the user account to receive the keys must already exist. It will automatically be created, of course, but not until very late in the checkout process. With paypal, it appears to occur after payment has been made. What's critical here is that once payment is made, conditional actions are set to advance the Payment Received order state to the Completed order state. In other words, by the time the account exists so that the product keys module can issue the keys, the order is already Completed (!)

Interestingly, there is simple logic which will make this work. The state changes will never be seen by the product keys module, but there is no apparent reason why product keys should care whether the state has changed. These hook implementations only fire when ubercart determines either some modification to the order, or checkout completes. They are rare events. Since the code already checks that keys are not issued twice, the simple solution is to eliminate all the extraneous logic of state changes entirely. Replacing the implementation of _uc_product_keys_order_ready() with this fixes the issue entirely:

function _uc_product_keys_order_ready($avOrder, $avStatus)
{
  $lvStatus = uc_order_status_data( $avStatus, 'state' ) ;
  
  if( $lvStatus == 'payment_received' || $lvStatus == 'completed' )
    return true ;
  else
    return false ;
}

Notably, I've stripped the module sub-hooking code that was present. Assuming that the logic was actually correct, that shouldn't have been causing any harm. (In the event of no sub-modules, it should be a no-op.) The more significant changes here are that there is no read of the old order state, and no comparison of the old order state with the new order state. All we care about is that the order has been paid for (or completed, which also implies payment). If that state condition is not met, keys should not be issued. Very simple yet effective logic.

If this logic works correctly for other payment systems as well, I recommend it be committed as the default logic. The only way I see it failing is if the payment system never causes the order to reach either Payment Received or Completed, but in that case it seems like a rather substantial flaw in the payment module itself.

freixas’s picture

freixas commented

Thanks for looking into this. My comments are:

  • I don't think stripping out the hook code is a good idea. This would require analyzing every payment module now in existence and every payment module that could be written in the future. There are some pretty weird ones out there and I like having a solution to offer people when they come up with a strange case.
  • The code change you made doesn't take into account orders that admins might create or modify. My initial try at this module didn't take that into account either. With your change, many of the orders which the admin modifies (to add a note, for example) are going to require a check to see if keys have been assigned. If it weren't for this, I'd probably go with your change (minus the hook strip). In addition, if you monitor the state transitions that the order goes through in hook_order(), you might discover that hook_order is called numerous times (for some payment modules) with the state set to payment received or completed, So even during checkout, you might wind up checking the order for assigned keys multiple times.

Probably what I should do is extend the order table to add a flag as to whether keys have been assigned or not (placing the info in a separate table would require a separate database access, which I'm trying to avoid). That change has to be done carefully and my time is pretty tight right now, so I probably won't be making it any time soon.

Your simplest solution (if you want to be able to update uc_product_keys in the future) would be to use the hook to implement your order ready strategy. Check to see how many times the keys are checked during order processing. If they are checked more than once, you can probably extend the $order object to add a flag to prevent further checks (this is simpler than modifying the database). As for the admin tests, you may not care if the admin edits are a little bit slower, so that may not be a problem for you.

I've marked this bug postponed. Since PayPal Standard is a commonly used payment system, I'd like to see it work without requiring any special work, but that will have to wait.

Thanks for bringing this to my attention!

lethallynx’s picture

lethallynx commented

Any updates on this freixas?

I am trying to allow anonymous users to purchase product keys.

What would you recommend I do to hot fix the problem? (Can't say I really understand this hook business ... )

Are you working on a fix?

I would be happy to donate some funds as this is a nice module =D

Cheers, Lynx.

freixas’s picture

freixas commented

Hi,

No I am not working on this and there are no updates on this issue.

Funds always prioritize my work. :-) If you're interested, email me through my contact form.

My recommendation for a "hot" fix is to use the hook I provided in order to issue keys whenever it's time.

jgrauman’s picture

jgrauman commented

Actually, kagerato's fix works for me too. I think his logic seems pretty solid. It even works on Admin created or updated orders since uc_product_keys already checks to see if keys are already assigned to an order before issuing new keys. I'll keep testing and let you know if anything comes up, but it seems to be working on the tests I've done (especially since I'm only using paypal right now so I don't have to worry about every payment module, but I can't imagine that one wouldn't end up in Payment Received or Completed).

Josh

freixas’s picture

freixas commented
Status: Active » Closed (won't fix)

I've decided to close this bug report.

The fix suggested by kagerato could just as easily be applied to the sub-module uc_product_keys_for_paypal or by creating an equivalent hook fix in a module of your own.

For instance, a module called uc_product_keys_kagerato would include:

function uc_product_keys_kagerato_uc_product_keys_order_ready($order, $status) {
    return $status  == 'payment_received' || $status == 'completed';
}