Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert to Order API for line item support #33

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

agileware-fj
Copy link
Contributor

This converts the integration to use the Order API to allow adding line items that are linked properly to the resulting contribution.

This might be related to or at least interfere with work on #32

@monishdeb
Copy link
Contributor

I have applied the patch on one of our staging site to test it correctly records separate line-item and and financial item on purchasing an order with multiple products. I have added an order with two product items:

  1. Test Product (National) - Green ($50)
  2. PEI T-Shirt ($18)

Here's the financial records before and after applying the patch:

BEFORE

civicrm_api3('LineItem', 'get', [
  'sequential' => 1,
  'contribution_id' => 25955,
  'api.FinancialItem.get' => ['entity_id' => "\$value.id", 'entity_table' => "civicrm_line_item"],
]);
// result
{

    "is_error": 0,
    "version": 3,
    "count": 2,
    "values": [
        {
            "id": "122651",
            "entity_table": "civicrm_contribution",
            "entity_id": "25955",
            "contribution_id": "25955",
            "price_field_id": "1",
            "label": "PEI T-Shirt",
            "qty": "1.00",
            "unit_price": "68.00",
            "line_total": "68.00",
            "financial_type_id": "60",
            "non_deductible_amount": "0.00",
            "contribution_type_id": "60",
            "api.FinancialItem.get": {
                "is_error": 0,
                "version": 3,
                "count": 0,
                "values": []
            }
        },
        {
            "id": "122652",
            "entity_table": "civicrm_contribution",
            "entity_id": "25955",
            "contribution_id": "25955",
            "price_field_id": "1",
            "label": "Test Product (National) - Green",
            "qty": "1.00",
            "unit_price": "68.00",
            "line_total": "68.00",
            "financial_type_id": "60",
            "non_deductible_amount": "0.00",
            "contribution_type_id": "60",
            "api.FinancialItem.get": {
                "is_error": 0,
                "version": 3,
                "count": 0,
                "values": []
            }
        }
    ]
}

AFTER

{

    "is_error": 0,
    "version": 3,
    "count": 2,
    "values": [
        {
            "id": "122665",
            "entity_table": "civicrm_contribution",
            "entity_id": "25960",
            "contribution_id": "25960",
            "price_field_id": "1",
            "label": "Test Product (National) - Green",
            "qty": "1.00",
            "unit_price": "68.00",
            "line_total": "68.00",
            "financial_type_id": "60",
            "non_deductible_amount": "0.00",
            "tax_amount": "0.00",
            "contribution_type_id": "60",
            "api.FinancialItem.get": {
                "is_error": 0,
                "version": 3,
                "count": 1,
                "id": 48868,
                "values": [
                    {
                        "id": "48868",
                        "created_date": "2021-01-20 03:12:24",
                        "transaction_date": "2021-01-20 03:12:23",
                        "contact_id": "49916",
                        "description": "Test Product (National) - Green",
                        "amount": "50.00",
                        "currency": "CAD",
                        "financial_account_id": "90",
                        "status_id": "1",
                        "entity_table": "civicrm_line_item",
                        "entity_id": "122665"
                    }
                ]
            }
        },
        {
            "id": "122666",
            "entity_table": "civicrm_contribution",
            "entity_id": "25960",
            "contribution_id": "25960",
            "price_field_id": "1",
            "label": "PEI T-Shirt",
            "qty": "1.00",
            "unit_price": "68.00",
            "line_total": "68.00",
            "financial_type_id": "60",
            "non_deductible_amount": "0.00",
            "tax_amount": "0.00",
            "contribution_type_id": "60",
            "api.FinancialItem.get": {
                "is_error": 0,
                "version": 3,
                "count": 1,
                "id": 48869,
                "values": [
                    {
                        "id": "48869",
                        "created_date": "2021-01-20 03:12:24",
                        "transaction_date": "2021-01-20 03:12:23",
                        "contact_id": "49916",
                        "description": "PEI T-Shirt",
                        "amount": "18.00",
                        "currency": "CAD",
                        "financial_account_id": "90",
                        "status_id": "1",
                        "entity_table": "civicrm_line_item",
                        "entity_id": "122666"
                    }
                ]
            }
        }
    ]
}

As you can see in the later case it fixes two issues:

  1. There is an associated FinancialItem which was missing earlier.
  2. LineItem total is correct

Hence, the current patch confirms that it correctly records the financial information on purchasing a product.

@monishdeb
Copy link
Contributor

monishdeb commented Jan 20, 2021

The current patch fixes another issue mentioned in #32 where I the update status code wasn't able to change the Donation's status to 'Cancelled' or 'Refunded' on cancelling or refunding a order. Here's the screenshot of both the cases of:

Cancelled contribution:
Screenshot 2021-01-20 at 1 57 07 PM

Refunded contribution:
Screenshot 2021-01-20 at 2 00 07 PM

Ensured that financial adjustment are correct.

'0' => 3,
),
$params['line_items'][0]['line_item'][] = array(
'price_field_id' => '1',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please replace the hardcoded ID with api call?

'trxn_id' => $txn_id,
'invoice_id' => $invoice_id,
'source' => $source,
'receive_date' => $order_paid_date,
'contribution_status_id' => $contribution_status_id,
// 'contribution_status_id' => $contribution_status_id,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like to know why this line is commented out!!.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably actually should be hard-coded to 'Pending' and then a payment recorded for completed WooCommerce orders, as per what the Order API expects.

@@ -677,7 +676,7 @@ public function add_contribution( $cid, &$order ) {
* @since 2.0
* @param array $params The params to be passsed to the API
*/
$contribution = civicrm_api3( 'Contribution', 'create', apply_filters( 'woocommerce_civicrm_contribution_create_params', $params ) );
$contribution = civicrm_api3( 'Order', 'create', apply_filters( 'woocommerce_civicrm_contribution_create_params', $params, $order ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 👍

Copy link

@ralphyke ralphyke Jan 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have put the new code in and it seems to fix the problem of the financial contribution link, but the plugin also seems to be trying to create a membership which is failing as follows:

Error Details

An error of type E_ERROR was caused in line 133 of the file /home/rjakn8hl475q/public_html/shop/wp-content/plugins/civicrm/civicrm/api/api.php. Error message: Uncaught CiviCRM_API3_Exception: [constraint violation: DB Error: constraint violation thrown

Maybe it is because I have set the WooCommerce product to set to Contribution Type = Membership in Civi although it has worked in the past. Can you tell me what the code is trying to do with creating a membership record in Civi because I can't see anywhere in WooCommerce where this is set or how it is triggered?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ralphyke yes, setting the Financial Type to Member Dues seems to (obscurely) attempt to create a Membership, I think I will strip that out for now as it's not the best implementation.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, if you can strip that out and then I can put in the new version again, that would be great

@mecachisenros
Copy link
Owner

Thanks @agileware-fj for the PRs and @monishdeb for reviewing.

I'll review and test and report back.

@JoeMurray
Copy link
Contributor

@monishdeb could you test for multiple line items to ensure that financial items are created properly in that use case.

Thanks, @mecachisenros for your work on this. JMA would like to leave the work and testing for the membership stuff to you and others as we focus on the financial stuff. Thanks again @agileware-fj for your patches. Cheers,

@agileware-justin
Copy link

This and the other two PRs were funded work as part of a WordPress / CiviCRM integration project.

We were planning on releasing these changes at some stage once the project was done, @JoeMurray your recent comment about maintainers and line items was the prompt to bring that plan forward :)

@monishdeb
Copy link
Contributor

@JoeMurray yes I have tested with multiple line-items and posted my findings above. I have tested with two items:

  1. Test Product (National) - Green ($50)
  2. PEI T-Shirt ($18)

And using chain API result, that the line-items and financial items are created correctly.

@mecachisenros mecachisenros merged commit cfcc0f4 into mecachisenros:master Feb 3, 2021
@mecachisenros
Copy link
Owner

@monishdeb @agileware-fj @agileware-justin @JoeMurray
Merged, with some fixes:

  • replaced hardcoded price_field_id with default_contribution_amount price field
  • line_items are created on a per product basis rather than one line_item with multiple products

@mecachisenros
Copy link
Owner

Forgot to mention, added basic Membership implementation.

The membership is configurable in the CiviCRM Settings panel in the Edit/Add Product screen in Woo.

All membership default/settings (i.e. period, plan, etc.) are based on how the Membership is configured in CiviCRM.

@monishdeb
Copy link
Contributor

Thanks you so much guys for getting this PR merged. Much needed change to get the financial entries recorded for the purchased order. Also enables line-item editor to edit the associated line-items properly now, unlike earlier which throws an error due to missing financial-item, which fixed by this patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants