mecachisenros / woocommerce_civicrm

GNU Affero General Public License v3.0
14 stars 16 forks source link

Contribution record not being created (similar to #24 et. al.) #46

Closed cdhassell closed 2 years ago

cdhassell commented 2 years ago

As some others have reported, the contribution record is not being created, but the sale does show up on the Woocommerce tab in CiviCRM. However this is a bit different. A snippet from the log is copied below - apologies for the length. The way it looks to me is, it hits "Not able to find contribution" but then throws a fatal error because the contribution line item already exists. I don't understand how that can happen.

WP version 5.8.1 CiviCRM 5.41.0 WC 5.8.0 PHP 7.3.22 MySQL 5.7.28

Edit: You can ignore the Debug statements below, I added those to class Woocommerce_CiviCRM_Manager while trying to understand the code.

Oct 28 00:29:27  [info] Not able to find contribution

Oct 28 00:29:27  [info] Not able to find contribution

Oct 28 00:29:28  [info] Debug: add_update_contact existing_emails for example1@aol.com found: Array
(
    [23588] => Array
        (
            [id] => 23588
            [contact_id] => 34514
            [location_type_id] => 1
            [email] => example1@aol.com
            [is_primary] => 1
            [is_billing] => 0
            [on_hold] => 0
            [is_bulkmail] => 0
        )

)

Oct 28 00:29:28  [info] Debug: add_update_contact processing email = Array
(
    [location_type_id] => 1
    [email] => example1@aol.com
    [contact_id] => 34514
    [id] => 23588
)

Oct 28 00:29:29  [info] Not able to find contribution

Oct 28 00:29:29  [error] $Fatal Error Details = Array
(
    [callback] => Array
        (
            [0] => CRM_Core_Error
            [1] => exceptionHandler
        )

    [code] => -5
    [message] => DB Error: already exists
    [mode] => 16
    [debug_info] => INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution_id` , `price_field_id` , `label` , `qty` , `unit_price` , `line_total` , `price_field_value_id` , `financial_type_id` , `tax_amount` ) VALUES ('civicrm_contribution' ,  6851 ,  6851 ,  9 , 'Princess Whatsername' ,  2 ,  15.00 ,  30.00 ,  14 ,  4 ,  0 )  [nativecode=1062 ** Duplicate entry 'civicrm_contribution-6851-6851-14-9' for key 'UI_line_item_value']
    [type] => DB_Error
    [user_info] => INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution_id` , `price_field_id` , `label` , `qty` , `unit_price` , `line_total` , `price_field_value_id` , `financial_type_id` , `tax_amount` ) VALUES ('civicrm_contribution' ,  6851 ,  6851 ,  9 , 'Princess Whatsername' ,  2 ,  15.00 ,  30.00 ,  14 ,  4 ,  0 )  [nativecode=1062 ** Duplicate entry 'civicrm_contribution-6851-6851-14-9' for key 'UI_line_item_value']
    [to_string] => [db_error: message="DB Error: already exists" code=-5 mode=callback callback=CRM_Core_Error::exceptionHandler prefix="" info="INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution_id` , `price_field_id` , `label` , `qty` , `unit_price` , `line_total` , `price_field_value_id` , `financial_type_id` , `tax_amount` ) VALUES ('civicrm_contribution' ,  6851 ,  6851 ,  9 , 'Princess Whatsername' ,  2 ,  15.00 ,  30.00 ,  14 ,  4 ,  0 )  [nativecode=1062 ** Duplicate entry 'civicrm_contribution-6851-6851-14-9' for key 'UI_line_item_value']"]
)

Oct 28 00:29:29  [debug] $backTrace = #0 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Core/Error.php(942): CRM_Core_Error::backtrace("backTrace", TRUE)
#1 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/vendor/pear/pear-core-minimal/src/PEAR.php(922): CRM_Core_Error::exceptionHandler(Object(DB_Error))
#2 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/vendor/pear/db/DB.php(997): PEAR_Error->__construct("DB Error: already exists", -5, 16, (Array:2), "INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution...")
#3 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/vendor/pear/pear-core-minimal/src/PEAR.php(575): DB_Error->__construct(-5, 16, (Array:2), "INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution...")
#4 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/vendor/pear/pear-core-minimal/src/PEAR.php(223): PEAR->_raiseError(Object(DB_mysqli), NULL, -5, 16, (Array:2), "INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution...", "DB_Error", TRUE)
#5 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/vendor/pear/db/DB/common.php(1928): PEAR->__call("raiseError", (Array:7))
#6 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/vendor/pear/db/DB/mysqli.php(936): DB_common->raiseError(-5, NULL, NULL, "INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution...", "1062 ** Duplicate entry 'civicrm_contribution-6851-6851-14-9' for key 'UI_lin...")
#7 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/vendor/pear/db/DB/mysqli.php(406): DB_mysqli->mysqliRaiseError()
#8 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/vendor/pear/db/DB/common.php(1234): DB_mysqli->simpleQuery("INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution...")
#9 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/packages/DB/DataObject.php(2696): DB_common->query("INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution...")
#10 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/packages/DB/DataObject.php(1245): DB_DataObject->_query("INSERT INTO `civicrm_line_item` (`entity_table` , `entity_id` , `contribution...")
#11 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Core/DAO.php(656): DB_DataObject->insert()
#12 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Price/BAO/LineItem.php(62): CRM_Core_DAO->save()
#13 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Price/BAO/LineItem.php(420): CRM_Price_BAO_LineItem::create((Array:11))
#14 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Contribute/BAO/Contribution.php(3651): CRM_Price_BAO_LineItem::processPriceSet(6851, (Array:1), Object(CRM_Contribute_BAO_Contribution), "civicrm_contribution", FALSE)
#15 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Contribute/BAO/Contribution.php(232): CRM_Contribute_BAO_Contribution::recordFinancialAccounts((Array:22))
#16 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Contribute/BAO/Contribution.php(496): CRM_Contribute_BAO_Contribution::add((Array:22))
#17 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/api/v3/utils.php(1314): CRM_Contribute_BAO_Contribution::create((Array:22), (Array:1))
#18 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/api/v3/Contribution.php(87): _civicrm_api3_basic_create("CRM_Contribute_BAO_Contribution", (Array:22), "Contribution")
#19 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/Civi/API/Provider/MagicFunctionProvider.php(89): civicrm_api3_contribution_create((Array:22))
#20 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/Civi/API/Kernel.php(149): Civi\API\Provider\MagicFunctionProvider->invoke((Array:8))
#21 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/Civi/API/Kernel.php(81): Civi\API\Kernel->runRequest((Array:8))
#22 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/api/api.php(132): Civi\API\Kernel->runSafe("Contribution", "create", (Array:16))
#23 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/api/v3/Order.php(169): civicrm_api3("Contribution", "create", (Array:16))
#24 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/Civi/API/Provider/MagicFunctionProvider.php(89): civicrm_api3_order_create((Array:15))
#25 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/Civi/API/Kernel.php(149): Civi\API\Provider\MagicFunctionProvider->invoke((Array:8))
#26 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/Civi/API/Kernel.php(81): Civi\API\Kernel->runRequest((Array:8))
#27 /home/user/wordpress/wp-content/plugins/civicrm/civicrm/api/api.php(132): Civi\API\Kernel->runSafe("Order", "create", (Array:11))
#28 /home/user/wordpress/wp-content/plugins/woocommerce_civicrm-master/includes/class-woocommerce-civicrm-manager.php(707): civicrm_api3("Order", "create", (Array:11))
#29 /home/user/wordpress/wp-content/plugins/woocommerce_civicrm-master/includes/class-woocommerce-civicrm-manager.php(108): Woocommerce_CiviCRM_Manager->add_contribution(34514, Object(Automattic\WooCommerce\Admin\Overrides\Order))
#30 /home/user/wordpress/wp-includes/class-wp-hook.php(303): Woocommerce_CiviCRM_Manager->action_order(15669, (Array:18), Object(Automattic\WooCommerce\Admin\Overrides\Order))
#31 /home/user/wordpress/wp-includes/class-wp-hook.php(327): WP_Hook->apply_filters("", (Array:3))
#32 /home/user/wordpress/wp-includes/plugin.php(470): WP_Hook->do_action((Array:3))
#33 /home/user/wordpress/wp-content/plugins/woocommerce/includes/class-wc-checkout.php(1183): do_action("woocommerce_checkout_order_processed", 15669, (Array:18), Object(Automattic\WooCommerce\Admin\Overrides\Order))
#34 /home/user/wordpress/wp-content/plugins/woocommerce/includes/class-wc-ajax.php(466): WC_Checkout->process_checkout()
#35 /home/user/wordpress/wp-includes/class-wp-hook.php(303): WC_AJAX::checkout("")
#36 /home/user/wordpress/wp-includes/class-wp-hook.php(327): WP_Hook->apply_filters("", (Array:1))
#37 /home/user/wordpress/wp-includes/plugin.php(470): WP_Hook->do_action((Array:1))
#38 /home/user/wordpress/wp-content/plugins/woocommerce/includes/class-wc-ajax.php(90): do_action("wc_ajax_checkout")
#39 /home/user/wordpress/wp-includes/class-wp-hook.php(303): WC_AJAX::do_wc_ajax("")
#40 /home/user/wordpress/wp-includes/class-wp-hook.php(327): WP_Hook->apply_filters(FALSE, (Array:1))
#41 /home/user/wordpress/wp-includes/plugin.php(470): WP_Hook->do_action((Array:1))
#42 /home/user/wordpress/wp-includes/template-loader.php(13): do_action("template_redirect")
#43 /home/user/wordpress/wp-blog-header.php(19): require_once("/home/user/wordpress/wp-includes/template-loader.php")
#44 /home/user/wordpress/index.php(17): require("/home/user/wordpress/wp-blog-header.php")
#45 {main}
cdhassell commented 2 years ago

Follow up: The fatal error refers to a mysql error #1062, Duplicate entry 'civicrm_contribution-6851-6851-14-9' for key 'UI_line_item_value'. That key is defined as:

ADD UNIQUE KEY UI_line_item_value (entity_table,entity_id,contribution_id,price_field_value_id,price_field_id),

So the key is a unique combination of the table name, entity ID, contribution ID, etc. But there is no such contribution ID as 6851, not in the civicrm_contribution table nor in civicrm_line_item. So why is this a duplicate? I took the step of optimizing the table in case some sort of inconsistency has crept into the index. But there was no change in behavior.

cdhassell commented 2 years ago

Further on this:

It appears that this error occurs whenever there is more than one kind of product in the Woocommerce cart. For example, a user buys 2 youth tickets and 3 adult tickets. This produces one row in the civicrm_line_item table for the youth tickets, and one row for the adult tickets. Both rows are tied to the same WC order ID, and therefore the same contribution_id. All of the indexed fields in the UI_line_item_value index will be identical in this case. Since the index is set for "UNIQUE" the insert of the second row will fail with a duplicate error.

When I alter the UI_line_item_value index to remove "UNIQUE" then it works. Obviously it is not a good solution to change the CiviCRM database schema! So it seems like the data from woocommerce_civicrm plugin are not formatted in a way that fits with the existing schema if there are multiple product types in the WC cart.

Not sure what to do at this point but I would like to hear from others if the above diagnosis makes sense or if there is another solution.

agileware-justin commented 2 years ago

@cdhassell great debugging and yes I think you are correct.

A few different ways to solve this issue:

  1. WooCommerce CiviCRM plugin could modify the CiviCRM table schema on install and alter the UI_line_item_value index to be it non-unique
  2. WooCommerce CiviCRM plugin could require that a CiviCRM Priceset be created and defined in the WooCommerce CiviCRM Settings, which would then be used for creating the line items, linking to the Priceset
  3. Alter the CiviCRM database schema, make the UI_line_item_value index non-unique

Have raised a PR to alter the CiviCRM database schema, see https://github.com/civicrm/civicrm-core/pull/22200

JoeMurray commented 2 years ago

You should not be trying to have two line items for the same price_set_field_id and price_set_field_value_id. Instead of multiple line items for the same field/option you should increase the qty.

JoeMurray commented 2 years ago

Probably what you really want is to have a price set with multiple fields if it is appropriate to have multiple line items. The WooCommerce integration should be assisting and requiring this. Happy to have a call to discuss if you want, @agileware-justin .

agileware-justin commented 2 years ago

@JoeMurray we went with option 3 as that was the easiest route. In doing so, it makes no sense from what we could see to have an index on those fields. Why would that be there?

Did you find any other possible solutions, different to the 3 outlined above?

JoeMurray commented 2 years ago

The supported approach is to create a distinct price field and/or price field option if there are to be more than one line item. I think that would correspond to 2 above.

agileware-justin commented 2 years ago

@JoeMurray Feel free to open a PR for review

JoeMurray commented 2 years ago

An advantage to doing it the supported way is then the financial information can be different for the different line items. For example, Donation and Membership could have different financial types in the price set configuration, leading to correct recording of different financial accounts in the civicrm_financial_item table.

agileware-justin commented 2 years ago

@JoeMurray seems to be doing the individual financial types just fine.

image

monishdeb commented 2 years ago

@agileware-justin have you applied the core patch in your CiviCRM setup to remove the line-item index? If yes, then it will allow recording items with the same/different financial types, although the unique index (UI) is on [entity-table,entity_id,contribution_id,price_field,price_field_value_id]. In this case, with the UI in place, it won't allow recording those 4 line-items of respective financial type as their priceField and priceFieldValue ID is the same - 1.

Extending Joe's comment above, I would say maybe we should have an additional setting on the WooCommerce Product Edit page against CiviCRM setting, where other than Financial Type and Membership type, we can have Price Field option to choose from? Like:

Screenshot 2022-02-03 at 9 15 04 AM

And maybe add a validation rule that will prevent more than one product to have a single price field option? If nothing selected then it will consider the default core price field option - 1

agileware-justin commented 2 years ago

@monishdeb why is there an index on those fields? Just to enforce uniqueness? Or some other practical reason?

JoeMurray commented 2 years ago

The index is to ensure that there is a separate line item for each price field / price field option combination.

agileware-justin commented 2 years ago

@JoeMurray @monishdeb thanks for the comments.

Consider this use case. There is an existing on-line shop, it has ~45,000 products in the catalog. CiviCRM is connected to the store to track contact details, purchases and send newsletters.

By modifying the table schema, removing the unique index on these fields; an order is able to be recorded in CiviCRM as a Contribution and all items in the order recorded as Line Items in the Contribution.

People placing orders using this on-line store can purchase anything from 1 item to 100 items or the entire catalog if they have the time and patience to click Add to Cart that many times. The sky is the limit.

Using this example above, I am not sure if what you are proposing @JoeMurray would then require:

  1. Manually assigning a CiviCRM Priceset in WooCommerce to each of the ~45,000 Products and then maintaining this going forward.
  2. Creating a unique CiviCRM Priceset for each of the ~45,000 products and then assigning it to each Product as per 1 above
  3. Using a single CiviCRM Priceset and having to add a CiviCRM Priceset Option for each of the current ~45,000 Products and then maintaining this going forward

Ideally, whatever the solution. It needs to:

  1. Be scalable
  2. Not require additional CiviCRM or WooCommerce administration on a per product basis
  3. Be simple to use

Good discussion.

christianwach commented 2 years ago

Interesting discussion folks... I don't have any answers to the set up that @agileware-justin describes (wow, that's a lot of products!) except to say that Integrate CiviCRM with WooCommerce has more than 100 hours of further development beyond what this plugin offers - including assigning a "Price Field Value" to Products... which solves the problem identified in this issue.

My suggestion for the scalability issue would be to create some kind of AJAX-driven dashlet on the Integrate CiviCRM with WooCommerce settings page that iterates through the Woo Products and updates them accordingly.

christianwach commented 2 years ago

more than 100 hours of further development

Indeed I reckon probably a great deal more time than that... Integrate CiviCRM with WooCommerce fixes every bug I could find in this plugin, adds support for WooCommerce Products that create Participants in CiviCRM, adds deduping and assigning Contact Sub-types to new Customers and much more. It's also just about ready to submit to the WordPress Plugin Directory. Just so you know.

cdhassell commented 2 years ago

Indeed I reckon that you are correct. I installed Integrate CiviCRM with WooCommerce instead and the issue is resolved. Thanks for all of your work Christian!

agileware-justin commented 2 years ago

Well that was worth the time commenting on this issue @christianwach - good work :smile: