gnucash maint: Multiple changes pushed
John Ralls
jralls at code.gnucash.org
Wed May 12 12:07:00 EDT 2021
Updated via https://github.com/Gnucash/gnucash/commit/f10b0339 (commit)
via https://github.com/Gnucash/gnucash/commit/da3c511b (commit)
via https://github.com/Gnucash/gnucash/commit/4f030aac (commit)
via https://github.com/Gnucash/gnucash/commit/18c72bad (commit)
via https://github.com/Gnucash/gnucash/commit/2b48fd37 (commit)
from https://github.com/Gnucash/gnucash/commit/d2a4aef3 (commit)
commit f10b0339fbe7ce2a5c071b049e42ed6f07aa8075
Merge: d2a4aef3b da3c511b6
Author: John Ralls <jralls at ceridwen.us>
Date: Wed May 12 09:01:32 2021 -0700
Merge John Ralls's 'bug798093bis' into maint.
commit da3c511b6c8f332ae883ab8fccdfec868f42d425
Author: John Ralls <jralls at ceridwen.us>
Date: Tue May 11 14:42:21 2021 -0700
Remove trading splits instead of trying to adjust them.
diff --git a/libgnucash/engine/Scrub.c b/libgnucash/engine/Scrub.c
index 3e4c97280..340cd3431 100644
--- a/libgnucash/engine/Scrub.c
+++ b/libgnucash/engine/Scrub.c
@@ -530,50 +530,6 @@ get_trading_split (Transaction *trans, Account *base,
return balance_split;
}
-/* Find the trading split for a commodity, but don't create any splits
- or accounts if they don't already exist. */
-static Split *
-find_trading_split (Transaction *trans, Account *root,
- gnc_commodity *commodity)
-{
- Account *trading_account;
- Account *ns_account;
- Account *account;
-
- if (!root)
- {
- root = gnc_book_get_root_account (xaccTransGetBook (trans));
- if (NULL == root)
- {
- /* This can't occur, things should be in books */
- PERR ("Bad data corruption, no root account in book");
- return NULL;
- }
- }
-
- trading_account = gnc_account_lookup_by_name (root, _("Trading"));
- if (!trading_account)
- {
- return NULL;
- }
-
- ns_account = gnc_account_lookup_by_name (trading_account,
- gnc_commodity_get_namespace(commodity));
- if (!ns_account)
- {
- return NULL;
- }
-
- account = gnc_account_lookup_by_name (ns_account,
- gnc_commodity_get_mnemonic(commodity));
- if (!account)
- {
- return NULL;
- }
-
- return xaccTransFindSplitByAccount(trans, account);
-}
-
static void
add_balance_split (Transaction *trans, gnc_numeric imbalance,
Account *root, Account *account)
@@ -631,71 +587,6 @@ gnc_transaction_balance_no_trading (Transaction *trans, Account *root,
}
}
-/** If there are existing trading splits, adjust the price or exchange
- rate in each of them to agree with the non-trading splits for the
- same commodity. If there are multiple non-trading splits for the
- same commodity in the transaction this will use the exchange rate in
- the last such split. This shouldn't happen, and if it does then there's
- not much we can do about it anyway.
-
- While we're at it, compute the value imbalance ignoring existing
- trading splits. */
-
-static gnc_numeric
-gnc_transaction_adjust_trading_splits (Transaction* trans, Account *root)
-{
- GList* splits;
- gnc_numeric imbalance = gnc_numeric_zero();
- for (splits = trans->splits; splits; splits = splits->next)
- {
- Split *split = splits->data;
- Split *balance_split = NULL;
- gnc_numeric value, amount;
- gnc_commodity *commodity, *txn_curr = xaccTransGetCurrency (trans);
-
- if (! xaccTransStillHasSplit (trans, split)) continue;
-
- commodity = xaccAccountGetCommodity (xaccSplitGetAccount(split));
- if (!commodity)
- {
- PERR("Split has no commodity");
- continue;
- }
-
- balance_split = find_trading_split (trans, root, commodity);
-
- if (balance_split != split)
- /* this is not a trading split */
- imbalance = gnc_numeric_add(imbalance, xaccSplitGetValue (split),
- GNC_DENOM_AUTO, GNC_HOW_DENOM_EXACT);
-
- /* Ignore splits where value or amount is zero */
- value = xaccSplitGetValue (split);
- amount = xaccSplitGetAmount (split);
- if (gnc_numeric_zero_p(amount) || gnc_numeric_zero_p(value))
- continue;
-
- if (balance_split && balance_split != split)
- {
- gnc_numeric convrate = gnc_numeric_div (amount, value,
- GNC_DENOM_AUTO, GNC_HOW_DENOM_REDUCE);
- gnc_numeric old_value, new_value;
- old_value = xaccSplitGetValue(balance_split);
- new_value = gnc_numeric_div (xaccSplitGetAmount(balance_split),
- convrate,
- gnc_commodity_get_fraction(txn_curr),
- GNC_HOW_RND_ROUND_HALF_UP);
- if (! gnc_numeric_equal (old_value, new_value))
- {
- xaccTransBeginEdit (trans);
- xaccSplitSetValue (balance_split, new_value);
- xaccSplitScrub (balance_split);
- xaccTransCommitEdit (trans);
- }
- }
- }
- return imbalance;
-}
static gnc_numeric
gnc_transaction_get_commodity_imbalance (Transaction *trans,
@@ -719,6 +610,42 @@ gnc_transaction_get_commodity_imbalance (Transaction *trans,
return val_imbalance;
}
+/* GFunc wrapper for xaccSplitDestroy */
+static void
+destroy_split (void* ptr, void* data)
+{
+ Split *split = GNC_SPLIT (ptr);
+ if (split)
+ xaccSplitDestroy (split);
+}
+
+/* Balancing transactions with trading accounts works best when
+ * starting with no trading splits.
+ */
+static void
+xaccTransClearTradingSplits (Transaction *trans)
+{
+ GList *trading_splits = NULL;
+
+ for (GList* node = trans->splits; node; node = node->next)
+ {
+ Split* split = GNC_SPLIT(node->data);
+ Account* acc = NULL;
+ if (!split)
+ continue;
+ acc = xaccSplitGetAccount(split);
+ if (acc && xaccAccountGetType(acc) == ACCT_TYPE_TRADING)
+ trading_splits = g_list_prepend (trading_splits, node->data);
+ }
+
+ if (!trading_splits)
+ return;
+
+ xaccTransBeginEdit (trans);
+ g_list_foreach (trading_splits, destroy_split, NULL);
+ xaccTransCommitEdit (trans);
+}
+
static void
gnc_transaction_balance_trading (Transaction *trans, Account *root)
{
@@ -859,8 +786,11 @@ xaccTransScrubImbalance (Transaction *trans, Account *root,
ENTER ("()");
- /* Must look for orphan splits even if there is no imbalance. */
+ /* Must look for orphan splits and remove trading splits even if
+ * there is no imbalance and we're not using trading accounts.
+ */
xaccTransScrubSplits (trans);
+ xaccTransClearTradingSplits (trans);
/* Return immediately if things are balanced. */
if (xaccTransIsBalanced (trans))
@@ -876,9 +806,7 @@ xaccTransScrubImbalance (Transaction *trans, Account *root,
return;
}
- imbalance = gnc_transaction_adjust_trading_splits (trans, root);
-
- /* Balance the value, ignoring existing trading splits */
+ imbalance = xaccTransGetImbalanceValue (trans);
if (! gnc_numeric_zero_p (imbalance))
{
PINFO ("Value unbalanced transaction");
commit 4f030aac6e8bb64aa687f82f92e11ab875a9bfdb
Author: John Ralls <jralls at ceridwen.us>
Date: Tue May 4 10:11:59 2021 -0700
Bug 798093 - Changing the symbol/abbreviation of a security...
after the trading account was created breaks GnuCash.
Revisited. The original changeset looked for a top level trading account
and a namespace account in the transaction currency; if either of those
accounts had been created in a different currency it would duplicate
them.
This commit will accept any such account regardless of commodity. If
more than one exists it will prefer the one in the root currency if
there is one, otherwise it will select the first one found.
diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp
index b82180d07..e828b661c 100644
--- a/libgnucash/engine/Account.cpp
+++ b/libgnucash/engine/Account.cpp
@@ -3120,26 +3120,31 @@ gnc_account_lookup_by_full_name (const Account *any_acc,
return found;
}
-Account*
+GList*
gnc_account_lookup_by_type_and_commodity (Account* root,
const char* name,
GNCAccountType acctype,
gnc_commodity* commodity)
{
+ GList *retval{};
auto rpriv{GET_PRIVATE(root)};
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 (xaccAccountGetType (account) == acctype)
{
+ if (commodity &&
+ !gnc_commodity_equiv(xaccAccountGetCommodity (account),
+ commodity))
+ continue;
+
if (name && strcmp(name, xaccAccountGetName(account)))
- continue; //name doesn't match so skip this one
+ continue;
- return account;
+ retval = g_list_prepend(retval, account);
}
}
- return nullptr;
+ return retval;
}
void
diff --git a/libgnucash/engine/Account.h b/libgnucash/engine/Account.h
index 1e2504f2e..04b6e6e07 100644
--- a/libgnucash/engine/Account.h
+++ b/libgnucash/engine/Account.h
@@ -938,24 +938,24 @@ Account *gnc_account_lookup_by_code (const Account *parent,
*/
Account *gnc_account_lookup_by_opening_balance (Account *account, gnc_commodity *commodity);
-/** Find a direct child account matching name, GNCAccountType, and commodity.
+/** Find a direct child account matching name, GNCAccountType, and/or commodity.
*
- * Note that commodity matching is by equivalence: If the
- * mnemonic/symbol and namespace are the same, it matches.
+ * Name and commodity may be nullptr in which case the accounts in the
+ * list may have any value for those properties. Note that commodity
+ * matching is by equivalence: If the mnemonic/symbol and namespace
+ * are the same, it matches.
*
* @param root The account among whose children one expects to find
* the account.
- * @param name The name of the account to look for. If nullptr the
- * returned account will match only on acctype and commodity.
+ * @param name The name of the account to look for or nullptr.
* @param acctype The GNCAccountType to match.
- * @param commodity The commodity in which the account should be denominated.
- * @return The book's trading account for the given commodity if
- * trading accounts are enabled and one exists; NULL otherwise.
+ * @param commodity The commodity in which the account should be denominated or nullptr.
+ * @return A GList of children matching the supplied parameters.
*/
-Account *gnc_account_lookup_by_type_and_commodity (Account* root,
- const char* name,
- GNCAccountType acctype,
- gnc_commodity* commodity);
+GList *gnc_account_lookup_by_type_and_commodity (Account* root,
+ const char* name,
+ GNCAccountType acctype,
+ gnc_commodity* commodity);
/** @} */
/* ------------------ */
diff --git a/libgnucash/engine/Scrub.c b/libgnucash/engine/Scrub.c
index 5e6e9c050..3e4c97280 100644
--- a/libgnucash/engine/Scrub.c
+++ b/libgnucash/engine/Scrub.c
@@ -51,6 +51,7 @@
#include "TransactionP.h"
#include "gnc-commodity.h"
#include "qofinstance-p.h"
+#include "gnc-session.h"
#undef G_LOG_DOMAIN
#define G_LOG_DOMAIN "gnc.engine.scrub"
@@ -442,43 +443,46 @@ get_balance_split (Transaction *trans, Account *root, Account *account,
return balance_split;
}
+static gnc_commodity*
+find_root_currency(void)
+{
+ QofSession *sess = gnc_get_current_session ();
+ Account *root = gnc_book_get_root_account (qof_session_get_book (sess));
+ gnc_commodity *root_currency = xaccAccountGetCommodity (root);
+
+ /* Some older books may not have a currency set on the root
+ * account. In that case find the first top-level INCOME account
+ * and use its currency. */
+ if (!root_currency)
+ {
+ GList *children = gnc_account_get_children (root);
+ for (GList *node = children; node && !root_currency;
+ node = g_list_next (node))
+ {
+ Account *child = GNC_ACCOUNT (node->data);
+ if (xaccAccountGetType (child) == ACCT_TYPE_INCOME)
+ root_currency = xaccAccountGetCommodity (child);
+ }
+ g_free (children);
+ }
+ return root_currency;
+}
+
/* Get the trading split for a given commodity, creating it (and the
- necessary accounts) if it doesn't exist. */
+ necessary parent accounts) if it doesn't exist. */
static Split *
-get_trading_split (Transaction *trans, Account *root,
+get_trading_split (Transaction *trans, Account *base,
gnc_commodity *commodity)
{
Split *balance_split;
Account *trading_account;
Account *ns_account;
Account *account;
- gnc_commodity *default_currency = NULL;
-
- if (!root)
- {
- root = gnc_book_get_root_account (xaccTransGetBook (trans));
- if (NULL == root)
- {
- /* This can't occur, things should be in books */
- PERR ("Bad data corruption, no root account in book");
- return NULL;
- }
- }
-
- /* Get the default currency. This is harder than it seems. It's not
- possible to call gnc_default_currency() since it's a UI function. One
- might think that the currency of the root account would do, but the root
- account has no currency. Instead look for the Income placeholder account
- and use its currency. */
- default_currency = xaccAccountGetCommodity(gnc_account_lookup_by_name(root,
- _("Income")));
- if (! default_currency)
- {
- default_currency = commodity;
- }
+ Account* root = gnc_book_get_root_account (xaccTransGetBook (trans));
+ gnc_commodity *root_currency = find_root_currency ();
trading_account = xaccScrubUtilityGetOrMakeAccount (root,
- default_currency,
+ NULL,
_("Trading"),
ACCT_TYPE_TRADING,
TRUE, FALSE);
@@ -489,7 +493,7 @@ get_trading_split (Transaction *trans, Account *root,
}
ns_account = xaccScrubUtilityGetOrMakeAccount (trading_account,
- default_currency,
+ NULL,
gnc_commodity_get_namespace(commodity),
ACCT_TYPE_TRADING,
TRUE, TRUE);
@@ -1451,42 +1455,91 @@ xaccAccountScrubColorNotSet (QofBook *book)
/* ================================================================ */
+static Account*
+construct_account (Account *root, gnc_commodity *currency, const char *accname,
+ GNCAccountType acctype, gboolean placeholder)
+{
+ gnc_commodity* root_currency = find_root_currency ();
+ Account *acc = xaccMallocAccount(gnc_account_get_book (root));
+ xaccAccountBeginEdit (acc);
+ if (accname && *accname)
+ xaccAccountSetName (acc, accname);
+ if (currency || root_currency)
+ xaccAccountSetCommodity (acc, currency ? currency : root_currency);
+ xaccAccountSetType (acc, acctype);
+ xaccAccountSetPlaceholder (acc, placeholder);
+
+ /* Hang the account off the root. */
+ gnc_account_append_child (root, acc);
+ xaccAccountCommitEdit (acc);
+ return acc;
+}
+
+static Account*
+find_root_currency_account_in_list (GList *acc_list)
+{
+ gnc_commodity* root_currency = find_root_currency();
+ for (GList *node = acc_list; node; node = g_list_next (node))
+ {
+ Account *acc = GNC_ACCOUNT (node->data);
+ gnc_commodity *acc_commodity = NULL;
+ if (G_UNLIKELY (!acc)) continue;
+ acc_commodity = xaccAccountGetCommodity(acc);
+ if (gnc_commodity_equiv (acc_commodity, root_currency))
+ return acc;
+ }
+
+ return NULL;
+}
+
+static Account*
+find_account_matching_name_in_list (GList *acc_list, const char* accname)
+{
+ for (GList* node = acc_list; node; node = g_list_next(node))
+ {
+ Account *acc = GNC_ACCOUNT (node->data);
+ if (G_UNLIKELY (!acc)) continue;
+ if (g_strcmp0 (accname, xaccAccountGetName(acc)))
+ {
+ g_list_free (acc_list);
+ return acc;
+ }
+ }
+ return NULL;
+}
+
Account *
xaccScrubUtilityGetOrMakeAccount (Account *root, gnc_commodity * currency,
const char *accname, GNCAccountType acctype,
gboolean placeholder, gboolean checkname)
{
- Account * acc;
+ GList* acc_list;
+ Account *acc = NULL;
g_return_val_if_fail (root, NULL);
- /* build the account name */
- if (!currency)
- {
- PERR ("No currency specified!");
- return NULL;
- }
+ acc_list =
+ gnc_account_lookup_by_type_and_commodity (root,
+ checkname ? accname : NULL,
+ acctype, currency);
- /* See if we've got one of these going already ... */
- acc = gnc_account_lookup_by_type_and_commodity (root,
- checkname ? accname : NULL,
- acctype, currency);
+ if (!acc_list)
+ return construct_account (root, currency, accname,
+ acctype, placeholder);
- if (acc == NULL)
+ if (g_list_next(acc_list))
{
- /* Guess not. We'll have to build one. */
- acc = xaccMallocAccount(gnc_account_get_book (root));
- xaccAccountBeginEdit (acc);
- xaccAccountSetName (acc, accname);
- xaccAccountSetCommodity (acc, currency);
- xaccAccountSetType (acc, acctype);
- xaccAccountSetPlaceholder (acc, placeholder);
-
- /* Hang the account off the root. */
- gnc_account_append_child (root, acc);
- xaccAccountCommitEdit (acc);
+ if (!currency)
+ acc = find_root_currency_account_in_list (acc_list);
+
+ if (!acc)
+ acc = find_account_matching_name_in_list (acc_list, accname);
}
+ if (!acc)
+ acc = GNC_ACCOUNT (acc_list->data);
+
+ g_list_free (acc_list);
return acc;
}
commit 18c72baddf3c200252ffd5eac3274351fcec42a7
Author: John Ralls <jralls at ceridwen.us>
Date: Tue May 4 13:20:01 2021 -0700
Don't bother scrubbing for orphans if there's no transaction currency.
If it found one it would try to create an orphan account with no
currency which will crash later.
diff --git a/libgnucash/engine/Scrub.c b/libgnucash/engine/Scrub.c
index c2607fcb6..5e6e9c050 100644
--- a/libgnucash/engine/Scrub.c
+++ b/libgnucash/engine/Scrub.c
@@ -110,6 +110,7 @@ TransScrubOrphansFast (Transaction *trans, Account *root)
if (!trans) return;
g_return_if_fail (root);
+ g_return_if_fail (trans->common_currency);
for (node = trans->splits; node; node = node->next)
{
commit 2b48fd375de12c1b4a6a896133a0fddd0aa92dbf
Author: John Ralls <jralls at ceridwen.us>
Date: Mon May 3 17:16:23 2021 -0700
Revert "Fix duplicate trading accounts."
This reverts commit ed486a58a45adcb90e6f1e5232b6d348bed83a4c.
diff --git a/libgnucash/engine/Account.cpp b/libgnucash/engine/Account.cpp
index d4b6b8215..b82180d07 100644
--- a/libgnucash/engine/Account.cpp
+++ b/libgnucash/engine/Account.cpp
@@ -3127,34 +3127,16 @@ 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))
{
- auto child{gnc_account_lookup_by_type_and_commodity(account, name,
- acctype,
- commodity)};
- if (child)
- return child;
+ if (name && strcmp(name, xaccAccountGetName(account)))
+ continue; //name doesn't match so skip this one
+
+ return account;
}
}
return nullptr;
Summary of changes:
libgnucash/engine/Account.cpp | 39 ++----
libgnucash/engine/Account.h | 24 ++--
libgnucash/engine/Scrub.c | 310 ++++++++++++++++++++----------------------
3 files changed, 171 insertions(+), 202 deletions(-)
More information about the gnucash-changes
mailing list