gnucash maint: Multiple changes pushed

[John Ralls]

Updated	 via  https://github.com/Gnucash/gnucash/commit/ed486a58 (commit)
	 via  https://github.com/Gnucash/gnucash/commit/2258e7a4 (commit)
	from  https://github.com/Gnucash/gnucash/commit/2e7fcad8 (commit)



commit ed486a58a45adcb90e6f1e5232b6d348bed83a4c
Author: John Ralls <jralls at ceridwen.us>
Date:   Sat Apr 24 15:44:08 2021 -0700

    Fix duplicate trading accounts.
    
    In many cases GnuCash would create a new trading account hierarchy
    when one already existed because gnc_account_lookup_by_type_and_commodity
    didn't check the account presented and didn't recurse down the
    account hierarchy correctly.

diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp
index b82180d07..d4b6b8215 100644
--- a/libgnucash/engine/Account.cpp
+++ b/libgnucash/engine/Account.cpp
@@ -3127,16 +3127,34 @@ gnc_account_lookup_by_type_and_commodity (Account* root,
                                           gnc_commodity* commodity)
 {
     auto rpriv{GET_PRIVATE(root)};
+    if (rpriv->type == acctype &&
+        gnc_commodity_equiv(rpriv->commodity, commodity))
+    {
+        if (name)
+        {
+            if (strcmp(name, rpriv->accountName) == 0)
+                return root;
+        }
+        else
+        {
+            return root;
+        }
+    }
+
+    /* Nope. Make sure the types are compatible */
+    if (!xaccAccountTypesCompatible(rpriv->type, acctype))
+        return nullptr;
+
+    /* Recurse */
     for (auto node = rpriv->children; node; node = node->next)
     {
         auto account{static_cast<Account*>(node->data)};
-        if (xaccAccountGetType (account) == acctype &&
-            gnc_commodity_equiv(xaccAccountGetCommodity (account), commodity))
         {
-            if (name && strcmp(name, xaccAccountGetName(account)))
-                continue; //name doesn't match so skip this one
-
-            return account;
+            auto child{gnc_account_lookup_by_type_and_commodity(account, name,
+                                                                acctype,
+                                                                commodity)};
+            if (child)
+                return child;
         }
     }
     return nullptr;

commit 2258e7a44ea550c709d9c429a973d66add31e5ec
Author: John Ralls <jralls at ceridwen.us>
Date:   Sat Apr 24 15:42:01 2021 -0700

    xaccAccountTypes was backwards
    
    vs. the documentation. The unit tests and dialog-account were similarly
    backwards, but the use in gnucash/import-export/ofx followed the docs.

diff --git a/gnucash/gnome-utils/dialog-account.c b/gnucash/gnome-utils/dialog-account.c
index 43f66a78a..508e191db 100644
--- a/gnucash/gnome-utils/dialog-account.c
+++ b/gnucash/gnome-utils/dialog-account.c
@@ -589,7 +589,7 @@ make_children_compatible (AccountWindow *aw)
     account = aw_get_account (aw);
     g_return_if_fail (account);
 
-    if (xaccAccountTypesCompatible (xaccAccountGetType (account), aw->type))
+    if (xaccAccountTypesCompatible (aw->type, xaccAccountGetType (account)))
         return;
 
     set_children_types (account, aw->type);
@@ -709,7 +709,7 @@ verify_children_compatible (AccountWindow *aw)
     if (!account)
         return FALSE;
 
-    if (xaccAccountTypesCompatible (xaccAccountGetType (account), aw->type))
+    if (xaccAccountTypesCompatible (aw->type, xaccAccountGetType (account)))
         return TRUE;
 
     if (gnc_account_n_children(account) == 0)
diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp
index f820cbdba..b82180d07 100644
--- a/libgnucash/engine/Account.cpp
+++ b/libgnucash/engine/Account.cpp
@@ -4509,8 +4509,17 @@ gboolean
 xaccAccountTypesCompatible (GNCAccountType parent_type,
                             GNCAccountType child_type)
 {
-    return ((xaccParentAccountTypesCompatibleWith (parent_type) &
-             (1 << child_type))
+    /* ACCT_TYPE_NONE isn't compatible with anything, even ACCT_TYPE_NONE. */
+    if (parent_type == ACCT_TYPE_NONE || child_type == ACCT_TYPE_NONE)
+        return FALSE;
+
+    /* ACCT_TYPE_ROOT can't have a parent account, and asking will raise
+     * an error. */
+    if (child_type == ACCT_TYPE_ROOT)
+        return FALSE;
+
+    return ((xaccParentAccountTypesCompatibleWith (child_type) &
+             (1 << parent_type))
             != 0);
 }
 
diff --git a/libgnucash/engine/test/utest-Account.cpp b/libgnucash/engine/test/utest-Account.cpp
index 30cb13723..35f411322 100644
--- a/libgnucash/engine/test/utest-Account.cpp
+++ b/libgnucash/engine/test/utest-Account.cpp
@@ -2251,9 +2251,8 @@ test_xaccAccountType_Compatibility (void)
     auto check2 = test_error_struct_new(logdomain, loglevel, msg2);
     gint loghandler;
 
-    for (type = ACCT_TYPE_BANK; type < NUM_ACCOUNT_TYPES; type = ++type)
+    for (type = ACCT_TYPE_BANK; type < NUM_ACCOUNT_TYPES; ++type)
     {
-        GNCAccountType child;
         if (type == ACCT_TYPE_ROOT)
         {
             loghandler = g_log_set_handler (logdomain, loglevel,
@@ -2278,11 +2277,11 @@ test_xaccAccountType_Compatibility (void)
             g_assert_cmpint (compat, == , equity_compat);
         else if (type == ACCT_TYPE_TRADING)
             g_assert_cmpint (compat, == , trading_compat);
-        for (child = ACCT_TYPE_NONE; child < ACCT_TYPE_LAST; child = ++child)
-            if (1 << child & compat)
-                g_assert (xaccAccountTypesCompatible (type, child));
+        for (auto parent = ACCT_TYPE_NONE; parent < ACCT_TYPE_LAST; ++parent)
+            if (1 << parent & compat)
+                g_assert (xaccAccountTypesCompatible (parent, type));
             else
-                g_assert (!xaccAccountTypesCompatible (type, child));
+                g_assert (!xaccAccountTypesCompatible (parent, type));
 
         compat = xaccAccountTypesCompatibleWith (type);
         if (type <= ACCT_TYPE_LIABILITY ||



Summary of changes:
 gnucash/gnome-utils/dialog-account.c     |  4 +--
 libgnucash/engine/Account.cpp            | 43 ++++++++++++++++++++++++++------
 libgnucash/engine/test/utest-Account.cpp | 11 ++++----
 3 files changed, 42 insertions(+), 16 deletions(-)