r23387 - gnucash/trunk/src - Collapse the two transaction currency scrubbing functions into one and fix some bugs.
Mike Alexander
mta at code.gnucash.org
Sun Nov 10 22:09:59 EST 2013
Author: mta
Date: 2013-11-10 22:09:57 -0500 (Sun, 10 Nov 2013)
New Revision: 23387
Trac: http://svn.gnucash.org/trac/changeset/23387
Modified:
gnucash/trunk/src/engine/Scrub.c
gnucash/trunk/src/engine/Scrub.h
gnucash/trunk/src/import-export/log-replay/gnc-log-replay.c
Log:
Collapse the two transaction currency scrubbing functions into one and fix some bugs.
The most serious bug was that it would, in some cases, set the transaction's currency
to a non-currency commodity. It also sometimes set the currency directly without calling
xaccTransSetCurrency which skipped a number of side effects.
Modified: gnucash/trunk/src/engine/Scrub.c
===================================================================
--- gnucash/trunk/src/engine/Scrub.c 2013-11-10 21:35:34 UTC (rev 23386)
+++ gnucash/trunk/src/engine/Scrub.c 2013-11-11 03:09:57 UTC (rev 23387)
@@ -294,78 +294,12 @@
Split *split = node->data;
Transaction *trans = xaccSplitGetParent(split);
- xaccTransScrubCurrencyFromSplits(trans);
+ xaccTransScrubCurrency(trans);
xaccTransScrubImbalance (trans, gnc_account_get_root (acc), NULL);
}
}
-void
-xaccTransScrubCurrencyFromSplits(Transaction *trans)
-{
- GList *node;
- gnc_commodity *common_currency = NULL;
-
- if (!trans) return;
-
- for (node = xaccTransGetSplitList (trans); node; node = node->next)
- {
- Split *split = node->data;
-
- if (!xaccTransStillHasSplit(trans, split)) continue;
- if (gnc_numeric_equal(xaccSplitGetAmount (split),
- xaccSplitGetValue (split)))
- {
-
- Account *s_account = xaccSplitGetAccount (split);
- gnc_commodity *s_commodity = xaccAccountGetCommodity (s_account);
-
- if (s_commodity)
- {
- if (gnc_commodity_is_currency(s_commodity))
- {
- /* Found a split where the amount is the same as the value and
- the commodity is a currency. If all splits in the transaction
- that fit this description are in the same currency then the
- transaction should be in that currency too. */
-
- if (common_currency == NULL)
- /* First one we've found, save the currency */
- common_currency = s_commodity;
- else if ( !gnc_commodity_equiv (common_currency, s_commodity))
- {
- /* Splits are inconsistent, more than one has a value equal to
- the amount, but they aren't all in the same currency. */
- common_currency = NULL;
- break;
- }
- }
- }
- }
- }
-
- if (common_currency &&
- !gnc_commodity_equiv (common_currency, xaccTransGetCurrency (trans)))
- {
-
- /* Found a common currency for the splits, and the transaction is not
- in that currency */
- gboolean trans_was_open;
-
- PINFO ("transaction in wrong currency");
-
- trans_was_open = xaccTransIsOpen (trans);
-
- if (!trans_was_open)
- xaccTransBeginEdit (trans);
-
- xaccTransSetCurrency (trans, common_currency);
-
- if (!trans_was_open)
- xaccTransCommitEdit (trans);
- }
-}
-
static Split *
get_balance_split (Transaction *trans, Account *root, Account *account,
gnc_commodity *commodity)
@@ -927,30 +861,9 @@
retval = FindCommonCurrency (trans->splits, ra, rb);
- /* Compare this value to what we think should be the 'right' value */
- if (!trans->common_currency)
- {
- trans->common_currency = retval;
- }
- else if (!gnc_commodity_equiv (retval, trans->common_currency))
- {
- char guid_str[GUID_ENCODING_LENGTH + 1];
- guid_to_string_buff(xaccTransGetGUID(trans), guid_str);
- PWARN ("expected common currency %s but found %s in txn %s\n",
- gnc_commodity_get_unique_name (trans->common_currency),
- gnc_commodity_get_unique_name (retval), guid_str);
- }
-
- if (NULL == retval)
- {
- /* In every situation I can think of, this routine should return
- * common currency. So make note of this ... */
- char guid_str[GUID_ENCODING_LENGTH + 1];
- guid_to_string_buff(xaccTransGetGUID(trans), guid_str);
- PWARN ("unable to find a common currency in txn %s, and that is strange.",
- guid_str);
- }
-
+ if (retval && !gnc_commodity_is_currency(retval))
+ retval = NULL;
+
return retval;
}
@@ -1018,11 +931,28 @@
g_return_val_if_fail (book, NULL);
+ /* Find the most commonly used currency among the splits. If a given split
+ is in a non-currency commodity, then look for an ancestor account in a
+ currency, but prefer currencies used directly in splits. Ignore trading
+ account splits in this whole process, they don't add any value to this algorithm. */
for (node = trans->splits; node; node = node->next)
{
Split *s = node->data;
+ unsigned int curr_weight;
+
if (s == NULL || s->acc == NULL) continue;
+ if (xaccAccountGetType(s->acc) == ACCT_TYPE_TRADING) continue;
com_scratch = xaccAccountGetCommodity(s->acc);
+ if (com_scratch && gnc_commodity_is_currency(com_scratch))
+ {
+ curr_weight = 3;
+ }
+ else
+ {
+ com_scratch = gnc_account_get_currency_or_parent(s->acc);
+ if (com_scratch == NULL) continue;
+ curr_weight = 1;
+ }
if ( comlist )
{
found = g_slist_find_custom(comlist, com_scratch, commodity_equal);
@@ -1031,13 +961,13 @@
{
CommodityCount *count = g_slice_new0(CommodityCount);
count->commodity = com_scratch;
- count->count = ( gnc_commodity_is_currency( com_scratch ) ? 3 : 2 );
+ count->count = curr_weight;
comlist = g_slist_append(comlist, count);
}
else
{
CommodityCount *count = (CommodityCount*)(found->data);
- count->count += ( gnc_commodity_is_currency( com_scratch ) ? 3 : 2 );
+ count->count += curr_weight;
}
}
found = g_slist_sort( comlist, commodity_compare);
@@ -1068,7 +998,7 @@
xaccTransScrubOrphans (trans);
currency = xaccTransGetCurrency (trans);
- if (currency) return;
+ if (currency && gnc_commodity_is_currency(currency)) return;
currency = xaccTransFindCommonCurrency (trans, qof_instance_get_book(trans));
if (currency)
@@ -1106,6 +1036,7 @@
}
}
}
+ return;
}
for (node = trans->splits; node; node = node->next)
@@ -1136,9 +1067,6 @@
* 'other' transaction, which is going to keep that
* information. So I don't bother with that here. -- cstim,
* 2002/11/20. */
- /* But if the commodity *isn't* a currency, then it's
- * the value that should be changed to the
- * amount. jralls, 2010-11-02 */
PWARN ("Adjusted split with mismatched values, desc=\"%s\" memo=\"%s\""
" old amount %s %s, new amount %s",
@@ -1147,14 +1075,7 @@
gnc_commodity_get_mnemonic (currency),
gnc_num_dbg_to_string (xaccSplitGetValue(sp)));
xaccTransBeginEdit (trans);
- if ( gnc_commodity_is_currency( currency))
- {
- xaccSplitSetAmount (sp, xaccSplitGetValue(sp));
- }
- else
- {
- xaccSplitSetValue(sp, xaccSplitGetAmount(sp));
- }
+ xaccSplitSetAmount (sp, xaccSplitGetValue(sp));
xaccTransCommitEdit (trans);
}
/*else
Modified: gnucash/trunk/src/engine/Scrub.h
===================================================================
--- gnucash/trunk/src/engine/Scrub.h 2013-11-10 21:35:34 UTC (rev 23386)
+++ gnucash/trunk/src/engine/Scrub.h 2013-11-11 03:09:57 UTC (rev 23387)
@@ -112,19 +112,12 @@
void xaccAccountTreeScrubImbalance (Account *acc);
/** The xaccTransScrubCurrency method fixes transactions without a
- * common_currency by using the old account currency and security
+ * common_currency by looking for the most commonly used currency
+ * among all the splits in the transaction. If this fails it falls
+ * back to using the old account currency and security
* fields of the parent accounts of the transaction's splits. */
void xaccTransScrubCurrency (Transaction *trans);
-/** The xaccTransScrubCurrencyFromSplits method fixes transactions
- * where the currency doesn't match the currency used in the splits
- * in the transaction. If all splits where the amount equals the
- * value and where the commodity is a currency have the same
- * currency, it sets the transaction's currency to that if it is
- * anything else. If the splits don't match that description the
- * transaction currency is not changed. */
-void xaccTransScrubCurrencyFromSplits(Transaction *trans);
-
/** The xaccAccountScrubCommodity method fixed accounts without
* a commodity by using the old account currency and security. */
void xaccAccountScrubCommodity (Account *account);
Modified: gnucash/trunk/src/import-export/log-replay/gnc-log-replay.c
===================================================================
--- gnucash/trunk/src/import-export/log-replay/gnc-log-replay.c 2013-11-10 21:35:34 UTC (rev 23386)
+++ gnucash/trunk/src/import-export/log-replay/gnc-log-replay.c 2013-11-11 03:09:57 UTC (rev 23387)
@@ -528,7 +528,7 @@
DEBUG("process_trans_record(): Record ended\n");
if (trans != NULL) /*If we played with a transaction, commit it here*/
{
- xaccTransScrubCurrencyFromSplits(trans);
+ xaccTransScrubCurrency(trans);
xaccTransSetReadOnly(trans, trans_ro);
xaccTransCommitEdit(trans);
g_free(trans_ro);
More information about the gnucash-changes
mailing list