Merge branch 'TXN_TYPE-is-dynamic' xaccTransGetTxnType into maint #1201

bindings/python/tests/test_transaction.py

@@ -112,15 +112,6 @@ class TestTransaction(TransactionSession):
         self.trans.ClearReadOnly()
         self.assertEqual( None, self.trans.GetReadOnly() )
 
-    def test_txntype(self):
-        self.assertEqual( '\x00', self.trans.GetTxnType() )
-        TYPE = 'I'
-        self.trans.SetTxnType(TYPE)
-        self.assertEqual( TYPE, self.trans.GetTxnType() )
-        TYPE = 'P'
-        self.trans.SetTxnType(TYPE)
-        self.assertEqual( TYPE, self.trans.GetTxnType() )
-
     def test_num(self):
         NUM = '5'
         self.assertEqual( '', self.trans.GetNum() )

libgnucash/engine/Transaction.c

@@ -279,6 +279,7 @@ gnc_transaction_init(Transaction* trans)
     trans->notes = (char*) is_unset;
     trans->doclink = (char*) is_unset;
     trans->void_reason = (char*) is_unset;
+    trans->txn_type = TXN_TYPE_UNCACHED;
     LEAVE (" ");
 }
 
@@ -1705,6 +1706,7 @@ xaccTransCommitEdit (Transaction *trans)
         qof_instance_set_dirty(QOF_INSTANCE(trans));
     }
 
+    trans->txn_type = TXN_TYPE_UNCACHED;
     qof_commit_edit_part2(QOF_INSTANCE(trans),
                           (void (*) (QofInstance *, QofBackendError))
                           trans_on_error,
@@ -2551,22 +2553,43 @@ xaccTransRetDateDue(const Transaction *trans)
 }
 
 char
-xaccTransGetTxnType (const Transaction *trans)
+xaccTransGetTxnType (Transaction *trans)
 {
-    const char *s = NULL;
-    GValue v = G_VALUE_INIT;
-    char ret = TXN_TYPE_NONE;
+    gboolean has_nonAPAR_amount = FALSE;
 
     if (!trans) return TXN_TYPE_NONE;
-    qof_instance_get_kvp (QOF_INSTANCE (trans), &v, 1, TRANS_TXN_TYPE_KVP);
-    if (G_VALUE_HOLDS_STRING (&v))
+
+    if (trans->txn_type != TXN_TYPE_UNCACHED)
+        return trans->txn_type;
+
+    trans->txn_type = TXN_TYPE_NONE;
+    for (GList *n = xaccTransGetSplitList (trans); n; n = g_list_next (n))
     {
-         s = g_value_get_string (&v);
-         if (s && strlen (s) == 1)
-             ret = s[0];
+        Account *acc = xaccSplitGetAccount (n->data);
+
+        if (!acc)
+            continue;
+
+        if (!xaccAccountIsAPARType (xaccAccountGetType (acc)) &&
+            !gnc_numeric_zero_p (xaccSplitGetValue (n->data)))
+            has_nonAPAR_amount = TRUE;
+        else if (trans->txn_type == TXN_TYPE_NONE)
+        {
+            GNCLot *lot = xaccSplitGetLot (n->data);
+            GncInvoice *invoice = gncInvoiceGetInvoiceFromLot (lot);
+            GncOwner owner;
+
+            if (invoice && trans == gncInvoiceGetPostedTxn (invoice))
+                trans->txn_type = TXN_TYPE_INVOICE;
+            else if (invoice || gncOwnerGetOwnerFromLot (lot, &owner))
+                trans->txn_type = TXN_TYPE_PAYMENT;
+        }
     }
-    g_value_unset (&v);
-    return ret;
+
+    if (!has_nonAPAR_amount && (trans->txn_type == TXN_TYPE_PAYMENT))
+        trans->txn_type = TXN_TYPE_LINK;
+
+    return trans->txn_type;
 }
 
 const char *

libgnucash/engine/Transaction.h

@@ -121,6 +121,7 @@ GType gnc_transaction_get_type(void);
 /** @name Transaction Type field values
 @{
 */
+#define TXN_TYPE_UNCACHED '?' /** Transaction type not yet cached */
 #define TXN_TYPE_NONE	 '\0' /**< No transaction type       */
 #define TXN_TYPE_INVOICE 'I'  /**< Transaction is an invoice */
 #define TXN_TYPE_PAYMENT 'P'  /**< Transaction is a payment  */
@@ -302,14 +303,21 @@ gboolean xaccTransUseTradingAccounts(const Transaction *trans);
  */
 void          xaccTransSortSplits (Transaction *trans);
 
-/** Set the  Transaction Type
+/** Set the Transaction Type: note the type will be saved into the
+ *  Transaction kvp property as a backward compatibility measure, for
+ *  previous GnuCash versions whose xaccTransGetTxnType reads from the
+ *  kvp slots.
  *
  * See #TXN_TYPE_NONE, #TXN_TYPE_INVOICE and #TXN_TYPE_PAYMENT */
 void	      xaccTransSetTxnType (Transaction *trans, char type);
-/** Returns the  Transaction Type
+
+/** Returns the Transaction Type: note this type will be derived from
+ * the transaction splits, returning #TXN_TYPE_NONE,
+ * #TXN_TYPE_INVOICE, #TXN_TYPE_LINK, or #TXN_TYPE_PAYMENT according
+ * to heuristics. It does not query the transaction kvp slots.
  *
  * See #TXN_TYPE_NONE, #TXN_TYPE_INVOICE and #TXN_TYPE_PAYMENT */
-char	      xaccTransGetTxnType (const Transaction *trans);
+char	      xaccTransGetTxnType (Transaction *trans);
 
 /** Sets the transaction Number (or ID) field; rather than use this function
  *  directly, see 'gnc_set_num_action' in engine/engine-helpers.c & .h which

libgnucash/engine/TransactionP.h

@@ -122,6 +122,8 @@ struct transaction_s
     char * void_reason;
     char * notes;
 
+    char txn_type;
+
     /* Cached bool value to indicate whether this is a closing txn. This is
      * cached from the KVP value because it is queried a lot. Tri-state value: -1
      * = uninitialized; 0 = FALSE, 1 = TRUE. */

libgnucash/engine/test/utest-Invoice.c

@@ -25,6 +25,7 @@
 #include <qof.h>
 #include <unittest-support.h>
 #include "../gncInvoice.h"
+#include "../Transaction.h"
 
 static const gchar *suitename = "/engine/gncInvoice";
 void test_suite_gncInvoice ( void );
@@ -48,7 +49,9 @@ typedef struct
     gnc_commodity *commodity;
 
     GncInvoice* invoice;
+    GncInvoice* invoice2;
     Transaction *trans;
+    Transaction *trans2;
 } Fixture;
 
 static void
@@ -66,16 +69,20 @@ setup( Fixture *fixture, gconstpointer pData )
 
     if (data->is_cust_doc)
     {
+        xaccAccountSetType (fixture->account2, ACCT_TYPE_RECEIVABLE);
         fixture->customer = gncCustomerCreate(fixture->book);
         gncOwnerInitCustomer(&fixture->owner, fixture->customer);
     }
     else
     {
+        xaccAccountSetType (fixture->account2, ACCT_TYPE_PAYABLE);
         fixture->vendor = gncVendorCreate(fixture->book);
         gncOwnerInitVendor(&fixture->owner, fixture->vendor);
     }
 
     fixture->invoice = gncInvoiceCreate(fixture->book);
+    fixture->invoice2 = NULL;
+    fixture->trans2 = NULL;
 }
 
 static void
@@ -143,12 +150,137 @@ setup_with_invoice( Fixture *fixture, gconstpointer pData )
     fixture->trans = gncInvoicePostToAccount(fixture->invoice, fixture->account2, ts1, ts2, "memo", TRUE, FALSE);
 }
 
+
+static void
+setup_with_invoice_and_payment (Fixture *fixture, gconstpointer pData)
+{
+    Split *split;
+    GNCLot *lot;
+    gnc_numeric amt = gnc_numeric_create (1000, 100);
+
+    /* 1. create invoice */
+    setup_with_invoice (fixture, pData);
+
+    /* 2. create payment */
+    fixture->trans2 = xaccMallocTransaction (fixture->book);
+    lot = gncInvoiceGetPostedLot (fixture->invoice);
+
+    xaccTransBeginEdit (fixture->trans2);
+    xaccTransSetCurrency (fixture->trans2, fixture->commodity);
+
+    /* This split will balance the invoice lot */
+    split = xaccMallocSplit (fixture->book);
+    xaccSplitSetParent (split, fixture->trans2);
+    xaccAccountBeginEdit (fixture->account2);
+    xaccSplitSetAccount (split, fixture->account2);
+    xaccSplitSetValue (split, gnc_numeric_neg (amt));
+    xaccSplitSetAmount (split, gnc_numeric_neg (amt));
+    xaccSplitSetLot (split, lot);
+
+    /* bank split will balance the transaction */
+    split = xaccMallocSplit (fixture->book);
+    xaccSplitSetParent (split, fixture->trans2);
+    xaccAccountBeginEdit (fixture->account);
+    xaccSplitSetAccount (split, fixture->account);
+    xaccSplitSetValue (split, amt);
+    xaccSplitSetAmount (split, amt);
+
+    xaccTransCommitEdit (fixture->trans2);
+    xaccAccountCommitEdit (fixture->account);
+    xaccAccountCommitEdit (fixture->account2);
+
+    gncInvoiceAutoApplyPayments (fixture->invoice);
+}
+
+static void
+setup_with_invoice_and_CN (Fixture *fixture, gconstpointer pData)
+{
+    const InvoiceData *data = (InvoiceData*) pData;
+
+    time64 ts1 = gnc_time(NULL);
+    time64 ts2 = ts1;
+    const char *desc = "Test description";
+    GncEntry *entry = NULL;
+    Split *split;
+    GNCLot *lot1, *lot2;
+    gnc_numeric amt = gnc_numeric_create (1000, 100);
+
+    setup (fixture, pData);
+
+    /* 1. invoice */
+    fixture->invoice = gncInvoiceCreate (fixture->book);
+    gncInvoiceSetCurrency (fixture->invoice, fixture->commodity);
+    gncInvoiceSetOwner (fixture->invoice, &fixture->owner);
+
+    entry = gncEntryCreate(fixture->book);
+    gncEntrySetDate (entry, ts1);
+    gncEntrySetDateEntered (entry, ts1);
+    gncEntrySetDescription (entry, desc);
+    gncEntrySetDocQuantity (entry, amt, FALSE);
+    gncEntrySetBillAccount (entry, fixture->account);
+    gncBillAddEntry (fixture->invoice, entry);
+
+    gncInvoicePostToAccount (fixture->invoice, fixture->account2, ts1, ts2, "memo", TRUE, FALSE);
+
+    /* 2. CN */
+    fixture->invoice2 = gncInvoiceCreate (fixture->book);
+    gncInvoiceSetCurrency (fixture->invoice2, fixture->commodity);
+    gncInvoiceSetOwner (fixture->invoice2, &fixture->owner);
+
+    entry = gncEntryCreate(fixture->book);
+    gncEntrySetDate (entry, ts1);
+    gncEntrySetDateEntered (entry, ts1);
+    gncEntrySetDescription (entry, desc);
+    gncEntrySetDocQuantity (entry, amt, TRUE);
+    gncEntrySetInvAccount(entry, fixture->account);
+    gncInvoiceAddEntry (fixture->invoice2, entry);
+
+    gncInvoicePostToAccount (fixture->invoice2, fixture->account2, ts1, ts2, "memo", TRUE, FALSE);
+
+    /* 3. now create the LL txn linking Invoice and CN */
+    lot1 = gncInvoiceGetPostedLot (fixture->invoice);
+    lot2 = gncInvoiceGetPostedLot (fixture->invoice2);
+    fixture->trans2 = xaccMallocTransaction (fixture->book);
+
+    xaccTransBeginEdit (fixture->trans2);
+    xaccTransSetCurrency (fixture->trans2, fixture->commodity);
+    xaccAccountBeginEdit (fixture->account2);
+
+    /* This split will balance the invoice */
+    split = xaccMallocSplit (fixture->book);
+    xaccSplitSetParent (split, fixture->trans2);
+    xaccSplitSetAccount (split, fixture->account2);
+    xaccSplitSetValue (split, gnc_numeric_neg (amt));
+    xaccSplitSetAmount (split, gnc_numeric_neg (amt));
+    xaccSplitSetLot (split, lot1);
+
+    /* This split will balance the CN*/
+    split = xaccMallocSplit (fixture->book);
+    xaccSplitSetParent (split, fixture->trans2);
+    xaccSplitSetAccount (split, fixture->account2);
+    xaccSplitSetValue (split, amt);
+    xaccSplitSetAmount (split, amt);
+    xaccSplitSetLot (split, lot2);
+
+    xaccTransCommitEdit (fixture->trans2);
+    xaccAccountCommitEdit (fixture->account2);
+}
+
 static void
 teardown_with_invoice( Fixture *fixture, gconstpointer pData )
 {
+    if (fixture->trans2)
+        xaccTransDestroy (fixture->trans2);
+
     gncInvoiceUnpost(fixture->invoice, TRUE);
     gncInvoiceRemoveEntries (fixture->invoice);
 
+    if (fixture->invoice2)
+    {
+        gncInvoiceUnpost(fixture->invoice2, TRUE);
+        gncInvoiceRemoveEntries (fixture->invoice2);
+    }
+
     teardown(fixture, pData);
 }
 
@@ -230,6 +362,25 @@ test_invoice_posted_trans ( Fixture *fixture, gconstpointer pData )
     }
 }
 
+// Testing for TXN_TYPE_INVOICE TXN_TYPE_PAYMENT are strictly testing functions
+// in Transaction.c, but require creating invoices, so, they are tested in
+// this file instead.
+static void
+test_xaccTransGetTxnTypeInvoice (Fixture *fixture, gconstpointer pData)
+{
+    g_assert_cmpint (TXN_TYPE_INVOICE, ==, xaccTransGetTxnType (fixture->trans));
+
+    g_assert_cmpint (TXN_TYPE_PAYMENT, ==, xaccTransGetTxnType (fixture->trans2));
+}
+
+
+static void
+test_xaccTransGetTxnTypeLink (Fixture *fixture, gconstpointer pData)
+{
+    g_assert_cmpint (TXN_TYPE_LINK, ==, xaccTransGetTxnType (fixture->trans2));
+}
+
+
 void
 test_suite_gncInvoice ( void )
 {
@@ -244,4 +395,8 @@ test_suite_gncInvoice ( void )
     GNC_TEST_ADD( suitename, "post trans - customer creditnote", Fixture, &pData, setup_with_invoice, test_invoice_posted_trans, teardown_with_invoice );
     pData.is_cn = FALSE;   // Customer invoice
     GNC_TEST_ADD( suitename, "post trans - customer invoice", Fixture, &pData, setup_with_invoice, test_invoice_posted_trans, teardown_with_invoice );
+
+    /* test txn type heuristics */
+    GNC_TEST_ADD( suitename, "tests txntype I & P", Fixture, &pData, setup_with_invoice_and_payment, test_xaccTransGetTxnTypeInvoice, teardown_with_invoice);
+    GNC_TEST_ADD( suitename, "tests txntype L", Fixture, &pData, setup_with_invoice_and_CN, test_xaccTransGetTxnTypeLink, teardown_with_invoice);
 }

libgnucash/engine/test/utest-Transaction.cpp

@@ -1838,13 +1838,12 @@ test_xaccTransGetReadOnly (Fixture *fixture, gconstpointer pData)
 static void
 test_xaccTransGetTxnType (Fixture *fixture, gconstpointer pData)
 {
-    const char i = 'I';
-    const char p = 'P';
+    // note this will only test TXN_TYPE_NONE, because TxnType is derived
+    // from split data. Testing for TXN_TYPE_INVOICE TXN_TYPE_PAYMENT
+    // will require creating invoices, so, they are tested in
+    // utest-Invoice.c
     auto txn = fixture->txn;
-    xaccTransSetTxnType(txn, i);
-    g_assert_cmpint (i, ==, xaccTransGetTxnType(txn));
-    xaccTransSetTxnType(txn, p);
-    g_assert_cmpint (p, ==, xaccTransGetTxnType(txn));
+    g_assert_cmpint (TXN_TYPE_NONE, ==, xaccTransGetTxnType(txn));
 }
 
 /* xaccTransGetReadOnly C: 7 in 5  Local: 1:0:0