IntenseFinance/gnucash
3
0
Fork 0
mirror of https://github.com/Gnucash/gnucash.git synced 2025-02-25 18:55:30 -06:00
Code Issues Packages Projects Releases Wiki Activity

Correctly re-value splits when the transaction currency is changed.

Browse Source 
When a transaction with existing splits had its currency changed, the
function would change the values to use the new currency's denominator
without changing the actual value. The balancing code would then apply
the price of the the new "other" split to the amount, changing it as
well. Changing the transaction currency back would convert the value in
the other split correctly so that it would equal the amount that the
balancing code wouldn't change anything. I actually detected this bug
when I wrote the test but didn't recognize it as a bug.

The new code first calculates a new price and then applies it to each
split so that the transaction balances correctly in the new transaction
currency. This also round-trips correctly
This commit is contained in:
John Ralls 2016-03-17 15:37:19 -07:00
parent c2ce204434
commit d0e103be08
2 changed files with 68 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/engine/Transaction.c
View File

@ -672,6 +672,8 @@ xaccTransCopyFromClipBoard(const Transaction *from_trans, Transaction *to_trans,
xaccTransBeginEdit(to_trans);
FOR_EACH_SPLIT(to_trans, xaccSplitDestroy(s));
g_list_free(to_trans->splits);
to_trans->splits = NULL;
xaccTransSetCurrency(to_trans, xaccTransGetCurrency(from_trans));
xaccTransSetDescription(to_trans, xaccTransGetDescription(from_trans));
@ -1267,22 +1269,78 @@ xaccTransGetCurrency (const Transaction *trans)
return trans ? trans->common_currency : NULL;
}
/* Helper functions for xaccTransSetCurrency */
static gnc_numeric
find_new_rate(Transaction *trans, gnc_commodity *curr)
{
GList *node;
gnc_numeric rate = gnc_numeric_zero();
for (node = trans->splits; node != NULL; node = g_list_next (node))
{
Split *split = GNC_SPLIT(node->data);
gnc_commodity *split_com =
xaccAccountGetCommodity(xaccSplitGetAccount(split));
if (gnc_commodity_equal(curr, split_com))
{
/* This looks backwards, but the amount of the balancing transaction
* that we're going to use it on is in the value's currency. */
rate = gnc_numeric_div(xaccSplitGetAmount(split),
xaccSplitGetValue(split),
GNC_DENOM_AUTO, GNC_HOW_RND_NEVER);
break;
}
}
return rate;
}
static void
split_set_new_value(Split* split, gnc_commodity *curr, gnc_commodity *old_curr,
gnc_numeric rate)
{
gnc_commodity *split_com =
xaccAccountGetCommodity(xaccSplitGetAccount(split));
if (gnc_commodity_equal(curr, split_com))
xaccSplitSetValue(split, xaccSplitGetAmount(split));
else if (gnc_commodity_equal(old_curr, split_com))
xaccSplitSetSharePrice(split, rate);
else
{
gnc_numeric old_rate = gnc_numeric_div(xaccSplitGetValue(split),
xaccSplitGetAmount(split),
GNC_DENOM_AUTO,
GNC_HOW_RND_NEVER);
gnc_numeric new_rate = gnc_numeric_div(old_rate, rate, GNC_DENOM_AUTO,
GNC_HOW_RND_NEVER);
xaccSplitSetSharePrice(split, new_rate);
}
}
/**
* Set a new currency on a transaction.
* When we do that to a transaction with splits we need to re-value
* all of the splits in the new currency.
* @param trans: The transaction to change
* @param curr: The new currency to set.
*/
void
xaccTransSetCurrency (Transaction *trans, gnc_commodity *curr)
{
gint fraction, old_fraction;
gnc_commodity *old_curr = trans->common_currency;
if (!trans || !curr || trans->common_currency == curr) return;
xaccTransBeginEdit(trans);
old_fraction = gnc_commodity_get_fraction (trans->common_currency);
trans->common_currency = curr;
fraction = gnc_commodity_get_fraction (curr);
/* avoid needless crud if fraction didn't change */
if (fraction != old_fraction)
if (old_curr != NULL && trans->splits != NULL)
{
FOR_EACH_SPLIT(trans, xaccSplitSetValue(s, xaccSplitGetValue(s)));
gnc_numeric rate = find_new_rate(trans, curr);
if (!gnc_numeric_zero_p (rate))
{
FOR_EACH_SPLIT(trans, split_set_new_value(s, curr, old_curr, rate));
}
else
{
FOR_EACH_SPLIT(trans, xaccSplitSetValue(s, xaccSplitGetValue(s)));
}
}
qof_instance_set_dirty(QOF_INSTANCE(trans));

6
src/engine/test/utest-Transaction.c
View File

@ -929,20 +929,18 @@ test_xaccTransEqual (Fixture *fixture, gconstpointer pData)
g_free (check->msg);
g_free (check2.msg);
check->msg = g_strdup("[xaccSplitEqual] amounts differ: 13333/1000 vs 100000/1000");
check2.msg = g_strdup_printf (
"[xaccTransEqual] splits %s and %s differ", split_guid0, split_guid0);
qof_instance_set_guid (split1, qof_instance_get_guid (split0));
g_assert (!xaccTransEqual (clone, txn0, TRUE, TRUE, TRUE, TRUE));
g_assert (xaccTransEqual (clone, txn0, TRUE, FALSE, FALSE, TRUE));
g_assert_cmpint (check->hits, ==, 11);
g_assert_cmpint (check->hits, ==, 10);
g_assert_cmpint (check2.hits, ==, 2);
qof_instance_set_guid (xaccTransGetSplit (txn1, 0),
qof_instance_get_guid (split0));
qof_instance_set_guid (xaccTransGetSplit (txn1, 1),
qof_instance_get_guid (xaccTransGetSplit (txn0, 1)));
g_free (check->msg);
{
Split* split00 = xaccTransGetSplit (txn0, 0);
Split* split01 = xaccTransGetSplit (txn0, 1);
@ -958,7 +956,7 @@ test_xaccTransEqual (Fixture *fixture, gconstpointer pData)
test_add_error (&check3);
g_assert (!xaccTransEqual (txn1, txn0, TRUE, TRUE, TRUE, TRUE));
g_assert (xaccTransEqual (txn1, txn0, TRUE, TRUE, FALSE, TRUE));
g_assert_cmpint (check->hits, ==, 12);
g_assert_cmpint (check->hits, ==, 11);
g_assert_cmpint (check2.hits, ==, 3);
g_assert_cmpint (check3.hits, ==, 0);