mirror of
https://github.com/Gnucash/gnucash.git
synced 2025-02-25 18:55:30 -06:00
Bugs 632346 & 632166: Fixup transaction currency scrubbing.
Biggest problem was that almost well-formed transactions missing a currency element wouldn't import correctly because the FindCommonCurrencies function only used the old currency values, ignoring the commodity values in the splits' accounts. A new function, xaccFindCommonCurrencies(), looks at those first and then calls xaccFindOldCommonCurrencies only if it fails. That addresses the cause of the import failure reported in 632166 and a big chunk of the warning messages complained about in 632346. A secondary problem, also addressed in this change, was that the last block in xaccTransScrubCurrencyFromSplits always replaced the "amount" with the "value". This is the right thing to do if the commodity for the split account is a currency, but if it isn't, the replacement should be reversed. git-svn-id: svn+ssh://svn.gnucash.org/repo/gnucash/trunk@19744 57a11ea4-9604-0410-9ed3-97b8803252fd
This commit is contained in:
parent
81cc53b59b
commit
1a9984eff5
@ -883,6 +883,12 @@ FindCommonExclSCurrency (SplitList *splits,
|
||||
int ab = !gnc_commodity_equiv(ra, sb);
|
||||
if ( aa && ab ) ra = NULL;
|
||||
}
|
||||
else if (!ra && rb)
|
||||
{
|
||||
int aa = !gnc_commodity_equiv(rb, sa);
|
||||
int ab = !gnc_commodity_equiv(rb, sb);
|
||||
ra = ( aa && ab ) ? NULL : rb;
|
||||
}
|
||||
|
||||
if ((!ra) && (!rb)) return NULL;
|
||||
}
|
||||
@ -948,6 +954,103 @@ xaccTransFindOldCommonCurrency (Transaction *trans, QofBook *book)
|
||||
return retval;
|
||||
}
|
||||
|
||||
/* Test the currency of the splits and find the most common and return
|
||||
* it, or NULL if there is no currency more common than the
|
||||
* others -- or none at all.
|
||||
*/
|
||||
typedef struct {
|
||||
gnc_commodity *commodity;
|
||||
unsigned int count;
|
||||
} CommodityCount;
|
||||
|
||||
static gint
|
||||
commodity_equal (gconstpointer a, gconstpointer b)
|
||||
{
|
||||
CommodityCount *cc = (CommodityCount*)a;
|
||||
gnc_commodity *com = (gnc_commodity*)b;
|
||||
if ( cc == NULL || cc->commodity == NULL ||
|
||||
!GNC_IS_COMMODITY( cc->commodity ) ) return -1;
|
||||
if ( com == NULL || !GNC_IS_COMMODITY( com ) ) return 1;
|
||||
if ( gnc_commodity_equal(cc->commodity, com) )
|
||||
return 0;
|
||||
return 1;
|
||||
}
|
||||
|
||||
static gint
|
||||
commodity_compare( gconstpointer a, gconstpointer b)
|
||||
{
|
||||
CommodityCount *ca = (CommodityCount*)a, *cb = (CommodityCount*)b;
|
||||
if (ca == NULL || ca->commodity == NULL ||
|
||||
!GNC_IS_COMMODITY( ca->commodity ) )
|
||||
{
|
||||
if (cb == NULL || cb->commodity == NULL ||
|
||||
!GNC_IS_COMMODITY( cb->commodity ) )
|
||||
return 0;
|
||||
return -1;
|
||||
}
|
||||
if (cb == NULL || cb->commodity == NULL ||
|
||||
!GNC_IS_COMMODITY( cb->commodity ) )
|
||||
return 1;
|
||||
if (ca->count == cb->count)
|
||||
return 0;
|
||||
return ca->count > cb->count ? 1 : -1;
|
||||
}
|
||||
|
||||
/* Find the commodities in the account of each of the splits of a
|
||||
* transaction, and rank them by how many splits in which they
|
||||
* occur. Commodities which are currencies count more than those which
|
||||
* aren't, because for simple buy and sell transactions it makes
|
||||
* slightly more sense for the transaction commodity to be the
|
||||
* currency -- to the extent that it makes sense for a transaction to
|
||||
* have a currency at all. jralls, 2010-11-02 */
|
||||
|
||||
static gnc_commodity *
|
||||
xaccTransFindCommonCurrency (Transaction *trans, QofBook *book)
|
||||
{
|
||||
gnc_commodity *com_first, *com_scratch;
|
||||
GList *node = NULL;
|
||||
GSList *comlist = NULL, *found = NULL;
|
||||
int score = 0;
|
||||
|
||||
if (!trans) return NULL;
|
||||
|
||||
if (trans->splits == NULL) return NULL;
|
||||
|
||||
g_return_val_if_fail (book, NULL);
|
||||
|
||||
for (node = trans->splits; node; node = node->next)
|
||||
{
|
||||
Split *s = node->data;
|
||||
if (s == NULL || s->acc == NULL) continue;
|
||||
com_scratch = xaccAccountGetCommodity(s->acc);
|
||||
if ( comlist )
|
||||
{
|
||||
found = g_slist_find_custom(comlist, com_scratch, commodity_equal);
|
||||
}
|
||||
if (comlist == NULL || found == NULL)
|
||||
{
|
||||
CommodityCount *count = g_slice_new0(CommodityCount);
|
||||
count->commodity = com_scratch;
|
||||
count->count = ( gnc_commodity_is_currency( com_scratch ) ? 3 : 2 );
|
||||
comlist = g_slist_append(comlist, count);
|
||||
}
|
||||
else
|
||||
{
|
||||
CommodityCount *count = (CommodityCount*)(found->data);
|
||||
count->count += ( gnc_commodity_is_currency( com_scratch ) ? 3 : 2 );
|
||||
}
|
||||
}
|
||||
found = g_slist_sort( comlist, commodity_compare);
|
||||
|
||||
if ( ((CommodityCount*)(found->data))->commodity != NULL)
|
||||
{
|
||||
return ((CommodityCount*)(found->data))->commodity;
|
||||
}
|
||||
/* We didn't find a currency in the current account structure, so try
|
||||
* an old one. */
|
||||
return xaccTransFindOldCommonCurrency( trans, book );
|
||||
}
|
||||
|
||||
/* ================================================================ */
|
||||
|
||||
void
|
||||
@ -967,7 +1070,7 @@ xaccTransScrubCurrency (Transaction *trans)
|
||||
currency = xaccTransGetCurrency (trans);
|
||||
if (currency) return;
|
||||
|
||||
currency = xaccTransFindOldCommonCurrency (trans, qof_instance_get_book(trans));
|
||||
currency = xaccTransFindCommonCurrency (trans, qof_instance_get_book(trans));
|
||||
if (currency)
|
||||
{
|
||||
xaccTransBeginEdit (trans);
|
||||
@ -1033,6 +1136,9 @@ 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",
|
||||
@ -1041,7 +1147,14 @@ xaccTransScrubCurrency (Transaction *trans)
|
||||
gnc_commodity_get_mnemonic (currency),
|
||||
gnc_num_dbg_to_string (xaccSplitGetValue(sp)));
|
||||
xaccTransBeginEdit (trans);
|
||||
xaccSplitSetAmount (sp, xaccSplitGetValue(sp));
|
||||
if ( gnc_commodity_is_currency( currency))
|
||||
{
|
||||
xaccSplitSetAmount (sp, xaccSplitGetValue(sp));
|
||||
}
|
||||
else
|
||||
{
|
||||
xaccSplitSetValue(sp, xaccSplitGetAmount(sp));
|
||||
}
|
||||
xaccTransCommitEdit (trans);
|
||||
}
|
||||
/*else
|
||||
@ -1055,6 +1168,7 @@ xaccTransScrubCurrency (Transaction *trans)
|
||||
}*/
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/* ================================================================ */
|
||||
|
Loading…
Reference in New Issue
Block a user