From 10e20f97c644997ba8697d5ff3c48ce4e48a5fb3 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Fri, 22 Jun 2018 15:23:06 +0800 Subject: [PATCH 1/4] [test-TR] add tests for reconcile report, date filter --- .../test/test-transaction.scm | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/gnucash/report/standard-reports/test/test-transaction.scm b/gnucash/report/standard-reports/test/test-transaction.scm index b60226ff98..1418987731 100644 --- a/gnucash/report/standard-reports/test/test-transaction.scm +++ b/gnucash/report/standard-reports/test/test-transaction.scm @@ -39,6 +39,7 @@ ;; copied from transaction.scm (define trep-uuid "2fe3b9833af044abb929a88d5a59620f") +(define reconcile-uuid "e45218c6d76f11e7b5ef0800277ef320") ;; Explicitly set locale to make the report output predictable (setlocale LC_ALL "C") @@ -64,6 +65,7 @@ (test-begin "transaction.scm") (null-test) (trep-tests) + (reconcile-tests) ;; (test-end) must be run as the last function, it will ;; return #f if any of the tests have failed. (test-end "transaction.scm")) @@ -860,3 +862,52 @@ (get-row-col sxml #f 6)))) (test-end "subtotal table") )) + +(define (reconcile-tests) + (let* ((env (create-test-env)) + (account-alist (env-create-account-structure-alist env structure)) + (bank (cdr (assoc "Bank" account-alist))) + (income (cdr (assoc "Income" account-alist))) + (liability (cdr (assoc "Liabilities" account-alist))) + (expense (cdr (assoc "Expenses" account-alist))) + (YEAR (gnc:time64-get-year (gnc:get-today))) + ) + + (define (options->sxml options test-title) + (gnc:options->sxml reconcile-uuid options "test-reconcile" test-title)) + + (define (default-testing-options) + (let ((options (gnc:make-report-options reconcile-uuid))) + (set-option! options "Accounts" "Accounts" (list bank liability)) + options)) + + ;; old transactions for testing reconcile date options + (env-transfer env 01 01 1970 bank expense 5 #:description "desc-1" #:num "trn1" #:memo "memo-3") + (env-transfer env 31 12 1969 income bank 10 #:description "desc-2" #:num "trn2" #:void-reason "void" #:notes "notes3") + (env-transfer env 31 12 1969 income bank 29 #:description "desc-3" #:num "trn3" + #:reconcile (cons #\c (gnc-dmy2time64 01 03 1970))) + (env-transfer env 01 02 1970 bank expense 15 #:description "desc-4" #:num "trn4" #:notes "notes2" #:memo "memo-1") + (env-transfer env 10 01 1970 liability expense 10 #:description "desc-5" #:num "trn5" #:void-reason "any") + (env-transfer env 10 01 1970 liability expense 11 #:description "desc-6" #:num "trn6" #:notes "notes1") + (env-transfer env 10 02 1970 bank expense 8 #:description "desc-7" #:num "trn7" #:notes "notes1" #:memo "memo-2" + #:reconcile (cons #\y (gnc-dmy2time64 01 03 1970))) + + + (let* ((options (default-testing-options))) + (let ((sxml (options->sxml options "null test"))) + (test-assert "sxml" + sxml)) + (set-option! options "General" "Start Date" (cons 'absolute (gnc-dmy2time64 01 03 1970))) + (set-option! options "General" "End Date" (cons 'absolute (gnc-dmy2time64 31 03 1970))) + (let ((sxml (options->sxml options "filter reconcile date"))) + (test-equal "test reconciled amounts = $8" + (list "Total For Reconciled" "-$8.00") + (get-row-col sxml 3 #f)) + (test-equal "test cleared amounts = $29" + (list "Total For Cleared" "$29.00") + (get-row-col sxml 6 #f)) + (test-equal "test unreconciled amounts = -$31" + (list "Total For Unreconciled" "-$31.00") + (get-row-col sxml 11 #f)) + sxml) + ))) From 3af9acec998403216873d55790428039e5cdb8bf Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Mon, 18 Jun 2018 10:36:11 +0800 Subject: [PATCH 2/4] Bug 796614 - Reconciliation report contains incorrect transactions This commit modifies the date filter for reconciliation report, ensuring only splits whose reconciled dates match report options. If a split is not yet reconciled, include it anyway. In a large datafile with a narrow reconciled date range, the datefilter is likely to be the filter with highest frequency of #f therefore it should be prioritised in the combined filter. I chose to add another optional keyword instead of reusing the existing #:custom-split-filter, because we need to detect when to override the QofQuery date filter. --- .../test/test-transaction.scm | 6 +- .../report/standard-reports/transaction.scm | 77 ++++++++++++++++--- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/gnucash/report/standard-reports/test/test-transaction.scm b/gnucash/report/standard-reports/test/test-transaction.scm index 1418987731..56f6ffc453 100644 --- a/gnucash/report/standard-reports/test/test-transaction.scm +++ b/gnucash/report/standard-reports/test/test-transaction.scm @@ -901,13 +901,13 @@ (set-option! options "General" "End Date" (cons 'absolute (gnc-dmy2time64 31 03 1970))) (let ((sxml (options->sxml options "filter reconcile date"))) (test-equal "test reconciled amounts = $8" - (list "Total For Reconciled" "-$8.00") + (list "Total For Reconciled" "$8.00") (get-row-col sxml 3 #f)) (test-equal "test cleared amounts = $29" (list "Total For Cleared" "$29.00") (get-row-col sxml 6 #f)) - (test-equal "test unreconciled amounts = -$31" - (list "Total For Unreconciled" "-$31.00") + (test-equal "test unreconciled amounts = $31" + (list "Total For Unreconciled" "$31.00") (get-row-col sxml 11 #f)) sxml) ))) diff --git a/gnucash/report/standard-reports/transaction.scm b/gnucash/report/standard-reports/transaction.scm index 11c3a74e5d..0a7a4a17cc 100644 --- a/gnucash/report/standard-reports/transaction.scm +++ b/gnucash/report/standard-reports/transaction.scm @@ -16,6 +16,7 @@ ;; and enable multiple data columns ;; - add support for indenting for better grouping ;; - add defaults suitable for a reconciliation report +;; including alternative date filtering strategy ;; - add subtotal summary grid ;; - by default, exclude closing transactions from the report ;; @@ -446,10 +447,44 @@ Credit Card, and Income accounts.")) (gnc:option-set-value (gnc:lookup-option options gnc:pagename-general optname-startdate) (cons 'relative 'start-prev-quarter)) (gnc:option-set-value (gnc:lookup-option options gnc:pagename-general optname-enddate) (cons 'relative 'today)) (gnc:option-set-value (gnc:lookup-option options gnc:pagename-display (N_ "Reconciled Date")) #t) - (gnc:option-set-value (gnc:lookup-option options gnc:pagename-display (N_ "Running Balance")) #t) + (gnc:option-set-value (gnc:lookup-option options gnc:pagename-display (N_ "Running Balance")) #f) (gnc:option-set-value (gnc:lookup-option options gnc:pagename-display (N_ "Memo")) #f) + (gnc:option-make-internal! options gnc:pagename-display "Running Balance") options) +(define reconcile-report-instructions + (gnc:make-html-text + (_ "The reconcile report is designed to be similar to the formal reconciliation tool. +Please select the account from Report Options. Please note the dates specified in the options +will apply to the Reconciliation Date.") + (gnc:html-markup-br) + (gnc:html-markup-br))) + +;; if split is reconciled, retrieve its reconciled date; if not yet reconciled, return #f +(define (split->reconcile-date split) + (and (char=? (xaccSplitGetReconcile split) #\y) + (xaccSplitGetDateReconciled split))) + +(define (reconcile-report-calculated-cells options) + (define (opt-val section name) + (gnc:option-value (gnc:lookup-option options section name))) + (letrec + ((split-amount (lambda (s) (if (gnc:split-voided? s) + (xaccSplitVoidFormerAmount s) + (xaccSplitGetAmount s)))) + (split-currency (lambda (s) (xaccAccountGetCommodity (xaccSplitGetAccount s)))) + (amount (lambda (s) (gnc:make-gnc-monetary (split-currency s) (split-amount s)))) + (debit-amount (lambda (s) (and (positive? (split-amount s)) + (amount s)))) + (credit-amount (lambda (s) (and (not (positive? (split-amount s))) + (gnc:monetary-neg (amount s)))))) + ;; similar to default-calculated-cells but disable dual-subtotals. + (list (vector (_ "Funds In") + debit-amount #f #t #f + (const "")) + (vector (_ "Funds Out") + credit-amount #f #t #f + (const ""))))) ;; ;; Default Transaction Report ;; @@ -1047,11 +1082,11 @@ be excluded from periodic reporting.") (add-if (column-uses? 'reconciled-date) (vector (_ "Reconciled Date") (lambda (split transaction-row?) - (gnc:make-html-table-cell/markup - "date-cell" - (if (eqv? (xaccSplitGetReconcile split) #\y) - (qof-print-date (xaccSplitGetDateReconciled split)) - ""))))) + (let ((reconcile-date (split->reconcile-date split))) + (and reconcile-date + (gnc:make-html-table-cell/markup + "date-cell" + (qof-print-date reconcile-date))))))) (add-if (column-uses? 'num) (vector (if (and BOOK-SPLIT-ACTION @@ -1764,13 +1799,20 @@ be excluded from periodic reporting.") ;; Here comes the renderer function for this report. -(define* (trep-renderer report-obj #:key custom-calculated-cells empty-report-message custom-split-filter) +(define* (trep-renderer report-obj #:key custom-calculated-cells empty-report-message + custom-split-filter split->date split->date-include-false?) ;; the trep-renderer is a define* function which, at minimum, takes the report object ;; ;; the optional arguments are: ;; #:custom-calculated-cells - a list of vectors to define customized data columns - ;; #:empty-report-message - a str which is displayed at the initial report opening + ;; #:empty-report-message - a str or html-object which is displayed at the initial report opening ;; #:custom-split-filter - a split->bool function to add to the split filter + ;; #:split->date - a split->time64 which overrides the default posted date filter + ;; (see reconcile report) + ;; #:split->date-include-false? - addendum to above, specifies filter behaviour if + ;; split->date returns #f. useful to include unreconciled splits in reconcile + ;; report. it can be useful for alternative date filtering, e.g. filter by + ;; transaction->invoice->payment date. (define options (gnc:report-options report-obj)) (define (opt-val section name) (gnc:option-value (gnc:lookup-option options section name))) @@ -1908,7 +1950,8 @@ be excluded from periodic reporting.") (qof-query-set-book query (gnc-get-current-book)) (xaccQueryAddAccountMatch query c_account_1 QOF-GUID-MATCH-ANY QOF-QUERY-AND) - (xaccQueryAddDateMatchTT query #t begindate #t enddate QOF-QUERY-AND) + (if (not split->date) + (xaccQueryAddDateMatchTT query #t begindate #t enddate QOF-QUERY-AND)) (case void-status ((non-void-only) (gnc:query-set-match-non-voids-only! query (gnc-get-current-book))) ((void-only) (gnc:query-set-match-voids-only! query (gnc-get-current-book))) @@ -1941,6 +1984,7 @@ be excluded from periodic reporting.") (set! splits (stable-sort! splits primary-comparator?)))) ;; Combined Filter: + ;; - include/exclude using split->date according to date options ;; - include/exclude splits to/from selected accounts ;; - substring/regex matcher for Transaction Description/Notes/Memo ;; - custom-split-filter, a split->bool function for derived reports @@ -1951,7 +1995,12 @@ be excluded from periodic reporting.") (if transaction-matcher-regexp (regexp-exec transaction-matcher-regexp str) (string-contains str transaction-matcher))))) - (and (case filter-mode + (and (or (not split->date) ; #f = ignore custom date filter + (let ((date (split->date split))) ; cache split->date time64 or #f. + (if date ; if a split->date exists, + (<= begindate date enddate) ; then check for inclusion; + split->date-include-false?))); else behave according to parameter + (case filter-mode ((none) #t) ((include) (is-filter-member split c_account_2)) ((exclude) (not (is-filter-member split c_account_2)))) @@ -2029,7 +2078,13 @@ be excluded from periodic reporting.") 'name (_ "Reconciliation Report") 'report-guid "e45218c6d76f11e7b5ef0800277ef320" 'options-generator reconcile-report-options-generator - 'renderer trep-renderer) + ;; the renderer is the same as trep, however we're using a different split-date strategy. + ;; we're comparing reconcile date for inclusion, and if split is unreconciled, include it anyway. + 'renderer (lambda (rpt) (trep-renderer rpt + #:custom-calculated-cells reconcile-report-calculated-cells + #:split->date split->reconcile-date + #:split->date-include-false? #t + #:empty-report-message reconcile-report-instructions))) ;; Define the report. (gnc:define-report From ae4b0bd871a95d47582b54064eac125230fee269 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Mon, 18 Jun 2018 10:37:28 +0800 Subject: [PATCH 3/4] [TR] apply custom-sort after filtering. As a follow on to last commit, if a large datafile is subject to a reconciled date filter, the initial QofQuery date matcher would be skipped, causing a large number of splits sent for custom sorting prior to filtering. It will always be more efficient that filtering is applied first. Therefore custom sorting should be applied after filtering. --- gnucash/report/standard-reports/transaction.scm | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/gnucash/report/standard-reports/transaction.scm b/gnucash/report/standard-reports/transaction.scm index 0a7a4a17cc..45dab5ac1d 100644 --- a/gnucash/report/standard-reports/transaction.scm +++ b/gnucash/report/standard-reports/transaction.scm @@ -1977,12 +1977,6 @@ be excluded from periodic reporting.") (qof-query-destroy query) - (if custom-sort? - (begin - (set! splits (stable-sort! splits date-comparator?)) - (set! splits (stable-sort! splits secondary-comparator?)) - (set! splits (stable-sort! splits primary-comparator?)))) - ;; Combined Filter: ;; - include/exclude using split->date according to date options ;; - include/exclude splits to/from selected accounts @@ -2013,6 +2007,11 @@ be excluded from periodic reporting.") ))) splits)) + (when custom-sort? + (set! splits (stable-sort! splits date-comparator?)) + (set! splits (stable-sort! splits secondary-comparator?)) + (set! splits (stable-sort! splits primary-comparator?))) + (if (null? splits) ;; error condition: no splits found From 010dd04e826adc8b24636eac61dab302685cb8e5 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Mon, 18 Jun 2018 14:46:45 +0800 Subject: [PATCH 4/4] [TR] move options-summary to appear above subtotal-table It seems more fitting that the order of items is: - title - options summary - subtotal table - main table --- gnucash/report/standard-reports/transaction.scm | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gnucash/report/standard-reports/transaction.scm b/gnucash/report/standard-reports/transaction.scm index 45dab5ac1d..f59bb520dd 100644 --- a/gnucash/report/standard-reports/transaction.scm +++ b/gnucash/report/standard-reports/transaction.scm @@ -2041,6 +2041,11 @@ be excluded from periodic reporting.") (qof-print-date begindate) (qof-print-date enddate))))) + (if (eq? infobox-display 'always) + (gnc:html-document-add-object! + document + (gnc:html-render-options-changed options))) + (if (and (opt-val gnc:pagename-display optname-grid) (if (memq primary-key DATE-SORTING-TYPES) (keylist-get-info date-subtotal-list primary-date-subtotal 'renderer-fn) @@ -2055,11 +2060,6 @@ be excluded from periodic reporting.") (gnc:html-document-add-object! document (grid->html-table grid list-of-rows list-of-cols)))) - (if (eq? infobox-display 'always) - (gnc:html-document-add-object! - document - (gnc:html-render-options-changed options))) - (gnc:html-document-add-object! document table))))) (gnc:report-finished)