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.

git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@23387 57a11ea4-9604-0410-9ed3-97b8803252fd
This commit is contained in:
Mike Alexander 2013-11-11 03:09:57 +00:00
parent 18ac55505b
commit 7942b9c11e
3 changed files with 30 additions and 116 deletions

View File

@ -294,78 +294,12 @@ xaccAccountScrubImbalance (Account *acc)
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 @@ xaccTransFindOldCommonCurrency (Transaction *trans, QofBook *book)
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 @@ xaccTransFindCommonCurrency (Transaction *trans, QofBook *book)
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 @@ xaccTransFindCommonCurrency (Transaction *trans, QofBook *book)
{
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 @@ xaccTransScrubCurrency (Transaction *trans)
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 @@ xaccTransScrubCurrency (Transaction *trans)
}
}
}
return;
}
for (node = trans->splits; node; node = node->next)
@ -1136,9 +1067,6 @@ xaccTransScrubCurrency (Transaction *trans)
* '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 @@ xaccTransScrubCurrency (Transaction *trans)
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

View File

@ -112,19 +112,12 @@ void xaccAccountScrubImbalance (Account *acc);
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);

View File

@ -528,7 +528,7 @@ static void process_trans_record( FILE *log_file)
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);