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

dev/core#193: Ensure that tax amount is calculated when checking for order API line item #13091

Merged
merged 1 commit into from
Nov 15, 2018
Merged

dev/core#193: Ensure that tax amount is calculated when checking for order API line item #13091

merged 1 commit into from
Nov 15, 2018

Conversation

omarabuhussein
Copy link
Member

@omarabuhussein omarabuhussein commented Nov 14, 2018

The PR Replaces the following :

Overview

When creating an Order using Order API a check is done to compare Contribution's total_amount against the sum of line item's line_total's. If Contribution total amount and Line Items' total are not equal, Order creation will fail.

Before

As we know:

The line item line_total do not include any tax. Tax will be recorded separately in line item tax_amount field.
The contribution total_amount do include tax. A separate contribution tax_amount field also records the amount of tax.

Since the validation only compares the sum of line_totals with contribution total_amount, whenever there is tax, the validation will fail.

For illustration consider following data set that should create 1 contribution with 2 line items for memberships.

order_data =
{
    'is_pay_later': True,
    'contact_id': 240L,
    'payment_instrument_id': 6,
    'line_items': [{
                       'line_item': {
                           '42': {
                               'price_field_value_id': 60L,
                               'price_field_id': 36L,
                               'entity_id': 1249L,
                               'tax_amount': 2.0,
                               'line_total': 10.0,
                               'financial_type_id': 2,
                               'label': 'General',
                               'entity_table': 'civicrm_membership',
                               'unit_price': 10,
                               'qty': '1'}}}, {
                       'line_item': {
                           '43': {
                               'price_field_value_id': 61L,
                               'price_field_id': 37L,
                               'entity_id': 1248L,
                               'tax_amount': 1.67,
                               'line_total': 8.33,
                               'financial_type_id': 2,
                               'label': 'Concession',
                               'entity_table': 'civicrm_membership',
                               'unit_price': '8.33',
                               'qty': '1'}}}],
    'tax_amount': 3.67,
    'total_amount': 22.0,
    'contribution_recur_id': 605L,
    'financial_type_id': 2,
    'fee_amount': 0,
    'payment_processor_id': 5L,
    'receive_date': u'2019-08-01',
    'contribution_status_id': 2}

Currently Calling the Order API to create Order with the above data results with "Line item total doesn't match with total amount". This is because total_amount (22.0) <> sum of line_totals (18.33)

After

The Line Items check now consider tax_amounts of line_items and order API works fine with the parameters above.

@civibot civibot bot added the master label Nov 14, 2018
@civibot
Copy link

civibot bot commented Nov 14, 2018

(Standard links)

@seamuslee001
Copy link
Contributor

ping @eileenmcnaughton @JoeMurray

@eileenmcnaughton
Copy link
Contributor

test this please

@monishdeb
Copy link
Member

monishdeb commented Nov 15, 2018

Merging on basis of:

@monishdeb monishdeb merged commit 4b939ca into civicrm:master Nov 15, 2018
@omarabuhussein
Copy link
Member Author

omarabuhussein commented Nov 15, 2018

thanks everyone

@omarabuhussein omarabuhussein deleted the dev/core#193-v2 branch November 15, 2018 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants