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

Bug 752035 - Transaction Report Filter By not Always Working

Browse Source 
Make sure the internal split function get_corr_account_split
behaves consistently on multi-split transactions. The transaction
report depends on this.

Add test case to catch potential regressions

Simplify filter test function in transaction report.
This commit is contained in:
Geert Janssens 2015-07-28 17:12:24 +02:00
parent 3ccaec6e38
commit 124a2479ef
6 changed files with 75 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
src/engine/Split.c
View File

@ -1383,40 +1383,18 @@ get_corr_account_split(const Split *sa, const Split **retval)
{
const Split *current_split;
GList *node;
gnc_numeric sa_value, current_value;
gboolean sa_value_positive, current_value_positive, seen_one = FALSE;
*retval = NULL;
g_return_val_if_fail(sa, FALSE);
sa_value = xaccSplitGetValue (sa);
sa_value_positive = gnc_numeric_positive_p(sa_value);
if (xaccTransCountSplits (sa->parent) > 2)
return FALSE;
for (node = sa->parent->splits; node; node = node->next)
{
current_split = node->data;
if (current_split == sa) continue;
if (!xaccTransStillHasSplit(sa->parent, current_split)) continue;
current_value = xaccSplitGetValue (current_split);
current_value_positive = gnc_numeric_positive_p(current_value);
if ((sa_value_positive && !current_value_positive) ||
(!sa_value_positive && current_value_positive))
{
if (seen_one)
{
*retval = NULL;
return FALSE;
}
else
{
*retval = current_split;
seen_one = TRUE;
}
}
}
return seen_one;
*retval = xaccSplitGetOtherSplit (sa);
if (*retval)
return TRUE;
else
return FALSE;
}
/* TODO: these static consts can be shared. */

6
src/engine/Split.h
View File

@ -441,6 +441,12 @@ int xaccSplitCompareOtherAccountCodes(const Split *sa, const Split *sb);
* were added for the transaction report, and is in C because the code
* was already written in C for the above functions and duplication
* is silly.
*
* Note that this will only return a real value in case of a
* two-split transaction as that is the only situation in which
* a reliable value can be returned. In other situations
* "-- Split Transaction --" will be returned as Account Name
* or "Split" for Account Code.
*/
char * xaccSplitGetCorrAccountFullName(const Split *sa);

26
src/engine/test/utest-Split.c
View File

@ -1265,10 +1265,15 @@ test_get_corr_account_split (Fixture *fixture, gconstpointer pData)
Split *split1 = xaccMallocSplit (book);
Split *split2 = xaccMallocSplit (book);
Split *split3 = xaccMallocSplit (book);
Split *split4 = xaccMallocSplit (book);
Split *split5 = xaccMallocSplit (book);
const Split *result = NULL;
const gnc_numeric factor = gnc_numeric_create (2, 1);
Account *acc1 = xaccMallocAccount (book);
Account *acc2 = xaccMallocAccount (book);
Account *acc3 = xaccMallocAccount (book);
Account *acc4 = xaccMallocAccount (book);
Account *acc5 = xaccMallocAccount (book);
#if defined(__clang__) && __clang_major__ < 6
#define _func "gboolean get_corr_account_split(const Split *, const Split **)"
#else
@ -1285,14 +1290,23 @@ test_get_corr_account_split (Fixture *fixture, gconstpointer pData)
xaccAccountSetCommodity (acc1, fixture->curr);
xaccAccountSetCommodity (acc2, fixture->curr);
xaccAccountSetCommodity (acc3, fixture->curr);
xaccAccountSetCommodity (acc4, fixture->curr);
xaccAccountSetCommodity (acc5, fixture->curr);
split1->acc = acc1;
split2->acc = acc2;
split3->acc = acc3;
split4->acc = acc4;
split5->acc = acc5;
split1->value = gnc_numeric_create (456, 240);
split2->value = gnc_numeric_neg (fixture->split->value);
split3->value = gnc_numeric_neg (split1->value);
split4->value = gnc_numeric_neg (gnc_numeric_mul (fixture->split->value,
factor,
GNC_DENOM_AUTO,
GNC_HOW_RND_NEVER));
split5->value = fixture->split->value;
g_assert (!fixture->func->get_corr_account_split(fixture->split, &result));
g_assert (result == NULL);
@ -1309,6 +1323,18 @@ test_get_corr_account_split (Fixture *fixture, gconstpointer pData)
xaccSplitSetParent (split3, txn);
xaccTransCommitEdit (txn);
g_assert (!fixture->func->get_corr_account_split(fixture->split, &result));
g_assert (result == NULL);
/* Test for bug 752035 */
xaccTransBeginEdit (txn);
xaccSplitSetParent (split1, NULL);
xaccSplitSetParent (split2, NULL);
xaccSplitSetParent (split3, NULL);
xaccSplitSetParent (split4, txn);
xaccSplitSetParent (split5, txn);
xaccTransCommitEdit (txn);
g_assert (!fixture->func->get_corr_account_split(fixture->split, &result));
g_assert (result == NULL);
g_assert_cmpint (check->hits, ==, 0);

1
src/report/report-system/report-system.scm
View File

@ -648,7 +648,6 @@
(export gnc:account-get-type-string-plural)
(export gnc:accounts-get-commodities)
(export gnc:get-current-account-tree-depth)
(export gnc:split-get-corr-account-full-name)
(export gnc:acccounts-get-all-subaccounts)
(export gnc:make-stats-collector)
(export gnc:make-drcr-collector)

3
src/report/report-system/report-utilities.scm
View File

@ -143,9 +143,6 @@
(let ((root (gnc-get-current-root-account)))
(gnc-account-get-tree-depth root)))
(define (gnc:split-get-corr-account-full-name split)
(xaccSplitGetCorrAccountFullName split))
;; Get all children of this list of accounts.
(define (gnc:acccounts-get-all-subaccounts accountlist)

67
src/report/standard-reports/transaction.scm
View File

@ -1378,40 +1378,45 @@ Credit Card, and Income accounts.")))))
name-sortkey name-subtotal name-date-subtotal
3 2))
(define (get-other-account-names account-list)
( map (lambda (acct) (gnc-account-get-full-name acct)) account-list))
;;(define (get-other-account-names account-list)
;; ( map (lambda (acct) (gnc-account-get-full-name acct)) account-list))
(define (is-filter-member split account-list splits-ok?)
(let ((fullname (gnc:split-get-corr-account-full-name split)))
(define (is-filter-member split account-list)
(let* ((txn (xaccSplitGetParent split))
(splitcount (xaccTransCountSplits txn)))
(if (string=? fullname (_ "-- Split Transaction --"))
;; Yep, this is a split transaction.
(cond
;; A 2-split transaction - test separately so it can be optimized
;; to significantly reduce the number of splits to traverse
;; in guile code
((= splitcount 2)
(let* ((other (xaccSplitGetOtherSplit split))
(other-acct (xaccSplitGetAccount other)))
(member other-acct account-list)))
(if splits-ok?
(let* ((txn (xaccSplitGetParent split))
(splits (xaccTransGetSplitList txn)))
;; A multi-split transaction - run over all splits
((> splitcount 2)
(let ((splits (xaccTransGetSplitList txn)))
;; Walk through the list of splits.
;; if we reach the end, return #f
;; if the 'this' != 'split' and the split->account is a member
;; of the account-list, then return #t, else recurse
(define (is-member splits)
(if (null? splits)
#f
(let* ((this (car splits))
(rest (cdr splits))
(acct (xaccSplitGetAccount this)))
(if (and (not (eq? this split))
(member acct account-list))
#t
(is-member rest)))))
;; Walk through the list of splits.
;; if we reach the end, return #f
;; if the 'this' != 'split' and the split->account is a member
;; of the account-list, then return #t, else recurse
(define (is-member splits)
(if (null? splits)
#f
(let* ((this (car splits))
(rest (cdr splits))
(acct (xaccSplitGetAccount this)))
(if (and (not (eq? this split))
(member acct account-list))
#t
(is-member rest)))))
(is-member splits))
#f)
(is-member splits)))
;; Nope, this is a regular transaction
(member fullname (get-other-account-names account-list))
)))
;; Single transaction splits
(else #f))))
(gnc:report-starting reportname)
@ -1467,7 +1472,7 @@ Credit Card, and Income accounts.")))))
(set! splits (qof-query-run query))
;;(gnc:warn "Splits in trep-renderer:" splits)
(gnc:warn "Splits in trep-renderer:" splits)
;;(gnc:warn "Filter account names:" (get-other-account-names c_account_2))
@ -1477,7 +1482,7 @@ Credit Card, and Income accounts.")))))
(begin
;;(gnc:warn "Including Filter Accounts")
(set! splits (filter (lambda (split)
(is-filter-member split c_account_2 #t))
(is-filter-member split c_account_2))
splits))
)
)
@ -1486,7 +1491,7 @@ Credit Card, and Income accounts.")))))
(begin
;;(gnc:warn "Excluding Filter Accounts")
(set! splits (filter (lambda (split)
(not (is-filter-member split c_account_2 #t)))
(not (is-filter-member split c_account_2)))
splits))
)
)