Duplicated registration but only paid once

Hi Dan,

Thanks for the clear replication steps, I'll try this out when I get a chance!

Cheers

Tom

We have just spotted what appears to be the same issue. Duplicate registrations that are exactly the same right down to the creation time, but (fortunately) only one order.

Any idea how to fix?

Cheers

Brett

Hi Brett,

This issue has popped up for me too, but I haven't been able to replicate this consistently.

Are there any clues as to why this has happened for you?

Cheers

Tom

Looks like what is happening is that when the user goes back via the back button, because they have previously submitted the form there is a cached version which does not have the registration ID in it. So when the form is submitted again it creates a new registration rather than updating the existing one.

I attach a patch which clears out any existing registrations on submission. An alternative would be to look in the $order->data['register_entities'] before clearing it, to see if any registrations already exist and obtain their IDs, but this would be harder to code and would as a consequence be more fragile, I think.

I'm able to reliably reproduce this issue using this scenario:
1) add a reg-enabled item to cart
2) increase quantity to 2 (or more)
3) proceed through checkout flow, supplying reg info for Person1 and Person2 in the registration form
4) before paying, return to the cart (or wherever you can modify quantity in your flow), and reduce quantity to 1
5) proceed through checkout flow again, this time supplying Person2 info (including email address) in the single registration form presented.
6) pay
7) look at the Order record, and see the 2 (or more) Registrations with duplicate info associated with the order, despite the order showing purchase of just 1 of a single product.

This scenario also allows a user to register multiple people while only paying for one ticket. I'm actively trying to figure out the best solution to prevent this scenario (if there's a rule that I should be using, please let me know!). Happy to test and report back on potential solutions.

Versions:
Commerce 7.x-1.11
Entity Reg 7.x-1.4
Commerce Reg 7.x-2.0-beta5+20-dev

Hi gobigmike,

Can I get you to test the attached patch for this issue on your scenario please?

I know it failed testing, but it's a pretty simple patch to apply if it has to be done manually.

Cheers

Tom

My patch was spun from the latest release, not the beta specified on this ticket, so that may be the cause.

The patch from #6 applied to my codebase without complaint. ("Hunk #1 succeeded at 284 with fuzz 1 (offset -18 lines).")

But in my setup, it creates a worse result than pre-patch: now, following the same scenario as in #9, no registrations get created with the final order. I see the Registrations were added to the order admin in step #4, above. But while everything appears fine on the user end in steps 5 & 6 (info for my 1 reg is displayed on screen as expected), through the admin I see that as soon as I hit the Review page that second time through, there are 0 registrations with the order, and none get added/created upon payment.

Maybe because in my config, the Registration Information pane is in the Checkout screen?

Yes, that's annoying. I'll see if I can reproduce it in our setup. Clearly there must be something I missed about the way that function works.

While you're working on that, I'm experimenting with a rule, triggered on line-item quantity change (then check if it's a reg-enabled product, and if the quantity decreased, then delete all registrations). Not yielding desired results yet; I'll update if I get it working.

Here's the current incarnation, which appears to be firing as desired, but not actually deleting the registrations:

{ "rules_reduce_reg_count_on_item_decrease" : {
    "LABEL" : "Reduce Reg count on item decrease",
    "PLUGIN" : "reaction rule",
    "OWNER" : "rules",
    "REQUIRES" : [ "commerce_registration", "rules", "commerce_rules_extra" ],
    "ON" : { "commerce_rules_extra_line_item_quantity_changed" : [] },
    "IF" : [
      { "commerce_registration_order_has_registration" : { "commerce_order" : [ "commerce-line-item" ] } },
      { "AND" : [] },
      { "data_is" : {
          "data" : [ "commerce-line-item-old-quantity" ],
          "op" : "\u003E",
          "value" : [ "commerce-line-item:quantity" ]
        }
      }
    ],
    "DO" : [
      { "commerce_registration_delete_registrations" : { "commerce_order" : [ "commerce_order" ] } },
    ]
  }
}

Ok, I think I have this working via Rules. From my previous post, I just updated the Action selector to be site:current-cart-order rather than commerce_order, and it started working as expected (all registrations in the cart get deleted when the reg event product's quantity is reduced, and the correct number of registrations get created on order completion).

So here's the rule I'm running:

{ "rules_remove_reg_on_item_quantity_decrease" : {
    "LABEL" : "Remove Registrations on Reg product line item quantity reduction",
    "PLUGIN" : "reaction rule",
    "OWNER" : "rules",
    "REQUIRES" : [ "commerce_registration", "rules", "commerce_rules_extra" ],
    "ON" : { "commerce_rules_extra_line_item_quantity_changed" : [] },
    "IF" : [
      { "commerce_registration_order_has_registration" : { "commerce_order" : [ "commerce-line-item" ] } },
      { "AND" : [] },
      { "data_is" : {
          "data" : [ "commerce-line-item-old-quantity" ],
          "op" : "\u003E",
          "value" : [ "commerce-line-item:quantity" ]
        }
      }
    ],
    "DO" : [
      { "commerce_registration_delete_registrations" : { "commerce_order" : [ "site:current-cart-order" ] } }
    ]
  }
}

Might make sense to add an additional AND condition to check for commerce-line-item:quantity > 0, because I believe commerce_reg is automatically going to delete all registrations when the line_item quantity drops to 0/is removed, right? If so, this rule would be running a duplicate commerce_registration_delete_registrations pass.

Happy to receive suggestions for improvements to this rule, or to hear that it's working for others...

I can confirm rule at #15 works from what I have tested. I will update this if I find anything that is not working with it.

The earlier patch at #6 seems to delete all the registrations on the order when I change the quantity and then causes problems when trying to add more registrations after that runs.

Didn't mean to hide the patch sorry

I looked into this some more and was able to see why the patch at #6 fails under some conditions, so I have rewritten it. New patch attached. In my testing it's proved robust so I have high hopes.

I did a little review of the patch. It seems you've added in much more then a fix for this issue. I wouldn't recommend committing this patch. Some of the issues I see is that token support for the titles of the registrations section have been removed, some untranslatable strings have been introduced, an alter hook was removed, the keys for the order registrations stored in the order's data has changed as well which would affect anyone who has used that info for other customizations.

So based on that I would say that the patch needs some work, but I applaud your attempts to get this working. Just remove all the changes that are not related to this issue and it would make it a better patch.

1. +++ b/includes/commerce_registration.checkout_pane.inc
   @@ -29,46 +28,23 @@ function commerce_registration_information_checkout_form($form, &$form_state, $c
   -      $prodkey = $line_item_id . 'prod-' . $product->sku->value();
   -      for ($k = 0; $k < $quantity; $k++) {
   +      $prodkey = 'prod-' . $product->sku->value();
   
   @@ -402,29 +372,18 @@ function commerce_registration_information_settings_form($checkout_pane) {
      $form['commerce_registration_product_title'] = array(
   ...
      $form['commerce_registration_registrant_help'] = array(
   

   You are changing the way the data is keyed. This will affect anyone who has used that data for any other customizations.

2. +++ b/includes/commerce_registration.checkout_pane.inc
   @@ -29,46 +28,23 @@ function commerce_registration_information_checkout_form($form, &$form_state, $c
   -        $title = t('@product - Registrant #@count', $title_vars);
   ...
   +        $title = check_plain($product->title->value() . ' - Registrant #' . $label);
   

   Good to sanitize, but this is no longer translatable, which is bad!

3. +++ b/includes/commerce_registration.checkout_pane.inc
   @@ -29,46 +28,23 @@ function commerce_registration_information_checkout_form($form, &$form_state, $c
   -            $title = token_replace($title, array(
   

   Why are you removing token support? You have done this in several places.

4. +++ b/includes/commerce_registration.checkout_pane.inc
   @@ -105,7 +81,7 @@ function commerce_registration_information_checkout_form($form, &$form_state, $c
   +          return $form;
   

   Are you sure?

5. +++ b/includes/commerce_registration.checkout_pane.inc
   @@ -153,22 +129,15 @@ function commerce_registration_information_checkout_form($form, &$form_state, $c
   -  // Allow other modules to alter the pane form.
   -  // Using hook_commerce_registration_information_checkout_form_alter().
   -  drupal_alter('commerce_registration_information_checkout_form', $pane_form, $form_state);
   

   Why are you removing a hook?

6. +++ b/includes/commerce_registration.checkout_pane.inc
   @@ -178,21 +147,20 @@ function commerce_registration_information_checkout_form($form, &$form_state, $c
   -        $prodkey = $line_item_id . 'prod-' . $product->sku->value();
   +      for ($i = 0; $i < $quantity; $i++) {
   +        $prodkey = 'prod-' . $product->sku->value();
   

   Again, probably a bad idea to change the data keys at this point.

7. +++ b/includes/commerce_registration.checkout_pane.inc
   @@ -208,7 +176,7 @@ function commerce_registration_information_checkout_form_validate($form, &$form_
   +            if (FALSE /*registration_is_registered($entity, $mail)*/) {
   

   Shouldn't force things.

Hmm, looks like that patch included some differences between versions not of my making. The actual change is only about 8 lines. Will take another look and see if I can figure out why it's picked up all the extra stuff.

I think it must have included another unrelated patch which was applied here to fix another issue. I'll re-create it when I get the chance, sorry for the confusion.

I've taken a somewhat different approach at fixing this issue. The patch attached adds a new rules action that loads all registrations related to a particular line item. The patch also adds two new default rules. One that removes all registration associated with a line item that is being deleted and another that removes an appropriate number of registrations when a line item quantity is reduced.

Also, there is an update that creates a variable so that by default the new rules are disabled for those people that are upgrading. Just in case they don't want to use this functionality.

OK, here's the patch I meant to supply.

While I appreciate that a Rules-based solution might fix the problem I don't think it is the right way to go. The principal justification for using Rules in modules is to allow the user to extend their functionality: in this case there is no such need, this being unambiguously a bug which needs to be fixed. Hopefully this version of the patch will prove acceptable as a fix.

Hi All,

I agree with Alf here that this is a bug and should be solved with a code fix rather than Rules.

Rules should be for extending functionality rather than used for bug fixes and workarounds for issues with modules.

Cheers

Tom

Completely agree. Your patch resolves this issue of deleting registrations after the quantity has been decreased when dealing with the cart. Reviewing the code it looks good. I haven't had a chance to test it yet though.

I think I was trying to kill 2 birds with one stone. Perhaps a discussion for another issue. There still remains an issue with what happens if an admin removes a line item with associated registrations from the order, or changes the quantity on the admin side. I would say that a rules based approach for the former at least would make sense since there may be circumstances where you may not want to delete all those registrations and instead change their status to cancelled or extend the functionality by sending the registrant a tailored email.

In the case of the change in quantity from the admin side, that functionality still needs work. You cannot actually manage registrations in a way that makes sense through the order edit UI yet.

I had a chance to test out the patch. It applies nicely and fixes the issue. The registrations are deleted once you progress past the registration checkout page. I think this makes sense since it gives you a chance to change the quantity back without loosing your existing registrations info if you may have screwed up.

Thanks for the patch, committed :)