From f5a7aeca791300d4e5942a28573ef0bd5d197dcd Mon Sep 17 00:00:00 2001 From: Geert Janssens Date: Fri, 25 Mar 2016 18:09:27 +0100 Subject: [PATCH] Bug 719904 - Decide payment type only based on the account type involved This commit improves payment type deduction by first checking whether an AR or AP account is found in the transaction. If not, fall back to previous heuristic of positive action means customer payment, negative action means vendor action. The logic can still go wrong (no means to declare an employee payment, and credit notes are interpreted as opposite sign payments). Needs a follow up to fix that. --- src/business/business-gnome/dialog-payment.c | 21 ++++++++++++++++--- .../business-gnome/gnc-plugin-business.c | 18 ++++++++++------ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/business/business-gnome/dialog-payment.c b/src/business/business-gnome/dialog-payment.c index 11b0c3ee9e..96d20e4a42 100644 --- a/src/business/business-gnome/dialog-payment.c +++ b/src/business/business-gnome/dialog-payment.c @@ -1170,19 +1170,34 @@ gnc_ui_payment_new (GncOwner *owner, QofBook *book) gboolean gnc_ui_payment_is_customer_payment(const Transaction *txn) { gboolean result = TRUE; - Split *assetaccount_split; + Split *assetaccount_split, *aparaccount_split; gnc_numeric amount; if (!txn) return result; - // We require the txn to have one split in an A/R or A/P account. - if (!xaccTransGetSplitList(txn)) return result; + + /* First test if one split is in an A/R or A/P account. + * That will give us the best Customer vs Vendor/Employee distinction */ + aparaccount_split = xaccTransGetFirstAPARAcctSplit(txn); + if (aparaccount_split) + { + if (xaccAccountGetType (xaccSplitGetAccount (aparaccount_split)) == ACCT_TYPE_RECEIVABLE) + return TRUE; // Type is Customer + else if (xaccAccountGetType (xaccSplitGetAccount (aparaccount_split)) == ACCT_TYPE_PAYABLE) + return FALSE; // Type is Vendor/Employee, there's not enough information to refine more + } + + /* For the lack of an A/R or A/P account we'll assume positive changes to an + * Asset/Liability or Equity account are Customer payments the others will be + * considered Vendor payments */ assetaccount_split = xaccTransGetFirstPaymentAcctSplit(txn); if (!assetaccount_split) { + /* Transaction isn't valid for a payment, just return the default + * Calling code will have to handle this situation properly */ g_message("No asset splits in txn \"%s\"; cannot use this for assigning a payment.", xaccTransGetDescription(txn)); return result; diff --git a/src/business/business-gnome/gnc-plugin-business.c b/src/business/business-gnome/gnc-plugin-business.c index 2c493f863a..b82a747214 100644 --- a/src/business/business-gnome/gnc-plugin-business.c +++ b/src/business/business-gnome/gnc-plugin-business.c @@ -822,7 +822,9 @@ static void gnc_plugin_business_cmd_assign_payment (GtkAction *action, SplitRegister *reg; Split *split; Transaction *trans; - gboolean is_customer; + gboolean have_owner; + GncOwner owner; + GncOwner *owner_p; g_return_if_fail (mw != NULL); g_return_if_fail (GNC_IS_PLUGIN_BUSINESS (mw->data)); @@ -846,16 +848,20 @@ static void gnc_plugin_business_cmd_assign_payment (GtkAction *action, trans = xaccSplitGetParent(split); g_return_if_fail(trans); - is_customer = gnc_ui_payment_is_customer_payment(trans); plugin_business = GNC_PLUGIN_BUSINESS (mw->data); plugin_business_priv = GNC_PLUGIN_BUSINESS_GET_PRIVATE (plugin_business); + have_owner = gncOwnerGetOwnerFromTxn (trans, &owner); + if (have_owner) + owner_p = &owner; + else if (gnc_ui_payment_is_customer_payment(trans)) + owner_p = plugin_business_priv->last_customer; + else + owner_p = plugin_business_priv->last_vendor; + gnc_business_assign_payment (gnc_plugin_page_get_window(plugin_page), - trans, - is_customer - ? plugin_business_priv->last_customer - : plugin_business_priv->last_vendor); + trans, owner_p); } static const gchar *register_txn_actions[] =