From 4e0dcec096c9704b12a205293d8c79b634c9c4af Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Sun, 1 Aug 2021 16:06:13 +1000 Subject: [PATCH] fix: areg: Explicitly avoid filtering by depth queries, perform valuation after all reportq filtering is done (in both the historical and recent postings), and make sure filtering is done at the correct point based on whether --txn-dates is set. (#1634) --- .../Reports/AccountTransactionsReport.hs | 46 ++++++++++--------- hledger/test/aregister.test | 37 ++++++++++----- 2 files changed, 50 insertions(+), 33 deletions(-) diff --git a/hledger-lib/Hledger/Reports/AccountTransactionsReport.hs b/hledger-lib/Hledger/Reports/AccountTransactionsReport.hs index 90970d64c..62254ed9a 100644 --- a/hledger-lib/Hledger/Reports/AccountTransactionsReport.hs +++ b/hledger-lib/Hledger/Reports/AccountTransactionsReport.hs @@ -96,8 +96,9 @@ accountTransactionsReport rspec@ReportSpec{_rsReportOpts=ropts} j thisacctq = it where -- a depth limit should not affect the account transactions report -- seems unnecessary for some reason XXX - reportq = simplifyQuery $ And [_rsQuery rspec, periodq, excludeforecastq (forecast_ ropts)] + reportq = simplifyQuery $ And [depthlessq, periodq, excludeforecastq (forecast_ ropts)] where + depthlessq = filterQuery (not . queryIsDepth) $ _rsQuery rspec periodq = Date . periodAsDateSpan $ period_ ropts -- Except in forecast mode, exclude future/forecast transactions. excludeforecastq (Just _) = Any @@ -109,40 +110,43 @@ accountTransactionsReport rspec@ReportSpec{_rsReportOpts=ropts} j thisacctq = it realq = filterQuery queryIsReal reportq statusq = filterQuery queryIsStatus reportq + journalValuation = \j' -> journalApplyValuationFromOptsWith rspec j' priceoracle + where priceoracle = journalPriceOracle (infer_value_ $ _rsReportOpts rspec) j + -- sort by the transaction's register date, for accurate starting balance - -- these are not yet filtered by tdate, we want to search them all for priorps transactions = ptraceAtWith 5 (("ts4:\n"++).pshowTransactions) . sortOn (transactionRegisterDate reportq thisacctq) . jtxns . ptraceAtWith 5 (("ts3:\n"++).pshowTransactions.jtxns) -- maybe convert these transactions to cost or value - . journalApplyValuationFromOpts rspec - . (if filtertxns then filterJournalTransactions reportq else id) + . journalValuation + -- If we haven't yet filtered by reportq, do so now. + $ (if txn_dates_ ropts then id else filterJournalTransactions reportq) journalAcctTxns + -- these are not yet filtered by tdate, we want to search them all for priorps + journalAcctTxns = + -- accountTransactionsReportItem will keep transactions of any date which + -- have any posting inside the report period. + -- Should we also require that transaction date is inside the report period ? + -- Should we be filtering by reportq here to apply other query terms (?) + -- Make it an option for now. + (if txn_dates_ ropts then filterJournalTransactions reportq else id) + . ptraceAtWith 5 (("ts2:\n"++).pshowTransactions.jtxns) + -- apply any cur:SYM filters in reportq + . (if queryIsNull symq then id else filterJournalAmounts symq) -- keep just the transactions affecting this account (via possibly realness or status-filtered postings) . traceAt 3 ("thisacctq: "++show thisacctq) - . ptraceAtWith 5 (("ts2:\n"++).pshowTransactions.jtxns) - . filterJournalTransactions thisacctq - . filterJournalPostings (And [realq, statusq]) . ptraceAtWith 5 (("ts1:\n"++).pshowTransactions.jtxns) - -- apply any cur:SYM filters in reportq - $ if queryIsNull symq then j else filterJournalAmounts symq j - where - -- accountTransactionsReportItem will keep transactions of any date which have any posting inside the report period. - -- Should we also require that transaction date is inside the report period ? - -- Should we be filtering by reportq here to apply other query terms (?) - -- Make it an option for now. - filtertxns = txn_dates_ ropts + . filterJournalTransactions thisacctq + $ filterJournalPostings (And [realq, statusq]) j startbal | balanceaccum_ ropts == Historical = sumPostings priorps - | otherwise = nullmixedamt + | otherwise = nullmixedamt where - priorps = dbg5 "priorps" $ - filter (matchesPosting - (dbg5 "priorq" $ - And [thisacctq, tostartdateq, datelessreportq])) - $ transactionsPostings transactions + priorps = dbg5 "priorps" . journalPostings . journalValuation $ + filterJournalPostings priorq journalAcctTxns + priorq = dbg5 "priorq" $ And [thisacctq, tostartdateq, datelessreportq] tostartdateq = case mstartdate of Just _ -> Date (DateSpan Nothing mstartdate) diff --git a/hledger/test/aregister.test b/hledger/test/aregister.test index a29b636ca..88e09b41a 100644 --- a/hledger/test/aregister.test +++ b/hledger/test/aregister.test @@ -1,3 +1,16 @@ +< +2021-01-01 + (b) 1 + +2021-01-02 + a 1 + b -1 + +# 1. aregister only shows transactions matching the account query, and displays other accounts involved +$ hledger -f- areg a +Transactions in a and subaccounts: +2021-01-02 b 1 1 + < 2021-01-01 (a) 1 @@ -8,8 +21,8 @@ 2021-01-03 (a:aa:aaa) 100 -# aregister ignores a depth limit, always showing transactions in subaccounts. #1448 -$ hledger -f- areg -w80 a depth:1 +# 2. aregister ignores a depth limit, always showing transactions in subaccounts. #1448 +$ hledger -f- areg a depth:1 Transactions in a and subaccounts: 2021-01-01 a 1 1 2021-01-02 a:aa 10 11 @@ -17,14 +30,14 @@ Transactions in a and subaccounts: #1634: -# # aregister is always in historical mode, showing balance from prior transactions. -# $ hledger -f- areg -w80 a -b 2021-01-02 -# Transactions in a and subaccounts: -# 2021-01-02 a:aa 10 11 -# 2021-01-03 a:aa:aaa 100 111 +# 3. aregister is always in historical mode, showing balance from prior transactions. +$ hledger -f- areg a -b 2021-01-02 +Transactions in a and subaccounts: +2021-01-02 a:aa 10 11 +2021-01-03 a:aa:aaa 100 111 -# # Any additional arguments are a query, filtering the transactions. This may -# # cause the running balance to diverge from the real-world running balance. -# $ hledger -f- areg -w80 a aaa -# Transactions in a and subaccounts: -# 2021-01-03 a:aa:aaa 100 100 +# 4. Any additional arguments are a query, filtering the transactions. This may +# cause the running balance to diverge from the real-world running balance. +$ hledger -f- areg a aaa +Transactions in a and subaccounts: +2021-01-03 a:aa:aaa 100 100