From 53611be6e9594e7ac54db95b91d8601105aeb928 Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Fri, 7 May 2021 20:20:47 +1000 Subject: [PATCH] lib,ui: Do all cost conversion and price stripping in journalSelectingAmountFromOpts. --- .../Reports/AccountTransactionsReport.hs | 60 ++++++++----------- hledger-lib/Hledger/Reports/EntriesReport.hs | 13 ++-- .../Hledger/Reports/MultiBalanceReport.hs | 22 +++---- hledger-lib/Hledger/Reports/PostingsReport.hs | 4 +- hledger-lib/Hledger/Reports/ReportOptions.hs | 9 +-- hledger-ui/Hledger/UI/TransactionScreen.hs | 5 +- 6 files changed, 54 insertions(+), 59 deletions(-) diff --git a/hledger-lib/Hledger/Reports/AccountTransactionsReport.hs b/hledger-lib/Hledger/Reports/AccountTransactionsReport.hs index 49b717f97..7f8ca6acd 100644 --- a/hledger-lib/Hledger/Reports/AccountTransactionsReport.hs +++ b/hledger-lib/Hledger/Reports/AccountTransactionsReport.hs @@ -84,45 +84,35 @@ accountTransactionsReport rspec@ReportSpec{rsOpts=ropts} j reportq thisacctq = i where -- a depth limit should not affect the account transactions report -- seems unnecessary for some reason XXX - reportq' = -- filterQuery (not . queryIsDepth) - reportq - - -- get all transactions - ts1 = - -- ptraceAtWith 5 (("ts1:\n"++).pshowTransactions) $ - jtxns j - - -- apply any cur:SYM filters in reportq' - symq = filterQuery queryIsSym reportq' - ts2 = - ptraceAtWith 5 (("ts2:\n"++).pshowTransactions) $ - (if queryIsNull symq then id else map (filterTransactionAmounts symq)) ts1 - - -- keep just the transactions affecting this account (via possibly realness or status-filtered postings) - realq = filterQuery queryIsReal reportq' - statusq = filterQuery queryIsStatus reportq' - ts3 = - traceAt 3 ("thisacctq: "++show thisacctq) $ - ptraceAtWith 5 (("ts3:\n"++).pshowTransactions) $ - filter (matchesTransaction thisacctq . filterTransactionPostings (And [realq, statusq])) ts2 - - -- maybe convert these transactions to cost or value - -- PARTIAL: - prices = journalPriceOracle (infer_value_ ropts) j - styles = journalCommodityStyles j + reportq' = reportq -- filterQuery (not . queryIsDepth) + symq = filterQuery queryIsSym reportq' + realq = filterQuery queryIsReal reportq' + statusq = filterQuery queryIsStatus reportq' + prices = journalPriceOracle (infer_value_ ropts) j + styles = journalCommodityStyles j periodlast = fromMaybe (error' "journalApplyValuation: expected a non-empty journal") $ -- XXX shouldn't happen reportPeriodOrJournalLastDay rspec j - tval = transactionApplyCostValuation prices styles periodlast (rsToday rspec) (cost_ ropts) $ value_ ropts - ts4 = - ptraceAtWith 5 (("ts4:\n"++).pshowTransactions) $ - map tval ts3 + pvalue = maybe id (postingApplyValuation prices styles periodlast (rsToday rspec)) $ value_ ropts -- 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 - ts5 = - ptraceAtWith 5 (("ts5:\n"++).pshowTransactions) $ - sortBy (comparing (transactionRegisterDate reportq' thisacctq)) ts4 + transactions = + ptraceAtWith 5 (("ts5:\n"++).pshowTransactions) + . sortBy (comparing (transactionRegisterDate reportq' thisacctq)) + . jtxns + -- maybe convert these transactions to cost or value + . ptraceAtWith 5 (("ts4:\n"++).pshowTransactions.jtxns) + . journalMapPostings pvalue + . journalSelectingAmountFromOpts ropts + -- keep just the transactions affecting this account (via possibly realness or status-filtered postings) + . traceAt 3 ("thisacctq: "++show thisacctq) + . ptraceAtWith 5 (("ts3:\n"++).pshowTransactions.jtxns) + . filterJournalTransactions thisacctq + . filterJournalPostings (And [realq, statusq]) + -- apply any cur:SYM filters in reportq' + . ptraceAtWith 5 (("ts2:\n"++).pshowTransactions.jtxns) + $ (if queryIsNull symq then id else filterJournalAmounts symq) j startbal | balancetype_ ropts == HistoricalBalance = sumPostings priorps @@ -132,7 +122,7 @@ accountTransactionsReport rspec@ReportSpec{rsOpts=ropts} j reportq thisacctq = i filter (matchesPosting (dbg5 "priorq" $ And [thisacctq, tostartdateq, datelessreportq])) - $ transactionsPostings ts5 + $ transactionsPostings transactions tostartdateq = case mstartdate of Just _ -> Date (DateSpan Nothing mstartdate) @@ -149,7 +139,7 @@ accountTransactionsReport rspec@ReportSpec{rsOpts=ropts} j reportq thisacctq = i items = reverse $ accountTransactionsReportItems reportq' thisacctq startbal maNegate $ (if filtertxns then filter (reportq' `matchesTransaction`) else id) $ - ts5 + transactions pshowTransactions :: [Transaction] -> String pshowTransactions = pshow . map (\t -> unwords [show $ tdate t, T.unpack $ tdescription t]) diff --git a/hledger-lib/Hledger/Reports/EntriesReport.hs b/hledger-lib/Hledger/Reports/EntriesReport.hs index 9da872834..491909f37 100644 --- a/hledger-lib/Hledger/Reports/EntriesReport.hs +++ b/hledger-lib/Hledger/Reports/EntriesReport.hs @@ -33,15 +33,18 @@ type EntriesReportItem = Transaction -- | Select transactions for an entries report. entriesReport :: ReportSpec -> Journal -> EntriesReport -entriesReport rspec@ReportSpec{rsOpts=ropts@ReportOpts{..}} j@Journal{..} = - sortBy (comparing getdate) $ filter (rsQuery rspec `matchesTransaction`) $ map tvalue jtxns +entriesReport rspec@ReportSpec{rsOpts=ropts@ReportOpts{..}} j = + sortBy (comparing getdate) . jtxns . filterJournalTransactions (rsQuery rspec) + . journalMapPostings pvalue + $ journalSelectingAmountFromOpts ropts{show_costs_=True} j where getdate = transactionDateFn ropts -- We may be converting posting amounts to value, per hledger_options.m4.md "Effect of --value on reports". - tvalue t@Transaction{..} = t{tpostings=map pvalue tpostings} + pvalue = maybe id (postingApplyValuation priceoracle styles periodlast (rsToday rspec)) value_ where - pvalue = postingApplyCostValuation (journalPriceOracle infer_value_ j) (journalCommodityStyles j) periodlast (rsToday rspec) cost_ value_ - where periodlast = fromMaybe (rsToday rspec) $ reportPeriodOrJournalLastDay rspec j + priceoracle = journalPriceOracle infer_value_ j + styles = journalCommodityStyles j + periodlast = fromMaybe (rsToday rspec) $ reportPeriodOrJournalLastDay rspec j tests_EntriesReport = tests "EntriesReport" [ tests "entriesReport" [ diff --git a/hledger-lib/Hledger/Reports/MultiBalanceReport.hs b/hledger-lib/Hledger/Reports/MultiBalanceReport.hs index d46330ae1..383417a15 100644 --- a/hledger-lib/Hledger/Reports/MultiBalanceReport.hs +++ b/hledger-lib/Hledger/Reports/MultiBalanceReport.hs @@ -248,8 +248,9 @@ getPostings :: ReportSpec -> Journal -> [(Posting, Day)] getPostings ReportSpec{rsQuery=query,rsOpts=ropts} = map (\p -> (p, date p)) . journalPostings . - filterJournalAmounts symq . -- remove amount parts excluded by cur: - filterJournalPostings reportq -- remove postings not matched by (adjusted) query + filterJournalAmounts symq . -- remove amount parts excluded by cur: + filterJournalPostings reportq . -- remove postings not matched by (adjusted) query + journalSelectingAmountFromOpts ropts where symq = dbg3 "symq" . filterQuery queryIsSym $ dbg3 "requested q" query -- The user's query with no depth limit, and expanded to the report span @@ -553,25 +554,24 @@ cumulativeSum :: (DateSpan -> Account -> Account) -> Account -> Map DateSpan Acc cumulativeSum value start = snd . M.mapAccumWithKey accumValued start where accumValued startAmt date newAmt = let s = sumAcct startAmt newAmt in (s, value date s) --- | Calculate the Posting and Account valuation functions required by this --- MultiBalanceReport. +-- | Calculate the Posting and Account valuation functions required by this MultiBalanceReport. postingAndAccountValuations :: ReportSpec -> Journal -> PriceOracle -> (DateSpan -> Posting -> Posting, DateSpan -> Account -> Account) postingAndAccountValuations ReportSpec{rsToday=today, rsOpts=ropts} j priceoracle = case value_ ropts of + -- If we're doing no valuation, just return the identity functions. + Nothing -> (const id, const id) -- If we're doing AtEnd valuation, we may need to value the same posting at different dates - -- (for example, when preparing a ValueChange report). So we should only convert to cost and - -- maybe strip prices from the Posting, and should do valuation on the Accounts. - Just v@(AtEnd _) -> (pvalue Nothing, avalue v) - -- Otherwise, all costing and valuation should be done on the Postings. - _ -> (pvalue (value_ ropts), const id) + -- (for example, when preparing a ValueChange report). So we should do valuation on the Accounts. + Just v@(AtEnd _) -> (const id, avalue v) + -- Otherwise, all valuation should be done on the Postings. + Just v -> (pvalue v, const id) where -- For a Posting: convert to cost, apply valuation, then strip prices if we don't need them (See issue #1507). - pvalue v span = maybeStripPrices . postingApplyCostValuation priceoracle styles (end span) today (cost_ ropts) v + pvalue v span = postingApplyValuation priceoracle styles (end span) today v -- For an Account: Apply valuation to both the inclusive and exclusive balances. avalue v span a = a{aibalance = value (aibalance a), aebalance = value (aebalance a)} where value = mixedAmountApplyValuation priceoracle styles (end span) today (error "multiBalanceReport: did not expect amount valuation to be called ") v -- PARTIAL: should not happen - maybeStripPrices = if show_costs_ ropts then id else postingStripPrices end = maybe (error "multiBalanceReport: expected all spans to have an end date") -- PARTIAL: should not happen (addDays (-1)) . spanEnd styles = journalCommodityStyles j diff --git a/hledger-lib/Hledger/Reports/PostingsReport.hs b/hledger-lib/Hledger/Reports/PostingsReport.hs index 25eb91ecc..9a53bbc13 100644 --- a/hledger-lib/Hledger/Reports/PostingsReport.hs +++ b/hledger-lib/Hledger/Reports/PostingsReport.hs @@ -76,9 +76,7 @@ postingsReport rspec@ReportSpec{rsOpts=ropts@ReportOpts{..}} j = items (precedingps, reportps) = matchedPostingsBeforeAndDuring rspec j reportspan -- We may be converting posting amounts to value, per hledger_options.m4.md "Effect of --value on reports". - -- Strip prices from postings if we won't need them. - pvalue periodlast = maybeStripPrices . postingApplyCostValuation priceoracle styles periodlast (rsToday rspec) cost_ value_ - where maybeStripPrices = if show_costs_ then id else postingStripPrices + pvalue periodlast = maybe id (postingApplyValuation priceoracle styles periodlast (rsToday rspec)) value_ -- Postings, or summary postings with their subperiod's end date, to be displayed. displayps :: [(Posting, Maybe Day)] diff --git a/hledger-lib/Hledger/Reports/ReportOptions.hs b/hledger-lib/Hledger/Reports/ReportOptions.hs index f72d23583..8608bde8a 100644 --- a/hledger-lib/Hledger/Reports/ReportOptions.hs +++ b/hledger-lib/Hledger/Reports/ReportOptions.hs @@ -488,13 +488,14 @@ flat_ = not . tree_ -- depthFromOpts :: ReportOpts -> Int -- depthFromOpts opts = min (fromMaybe 99999 $ depth_ opts) (queryDepth $ queryFromOpts nulldate opts) --- | Convert this journal's postings' amounts to cost using their --- transaction prices, if specified by options (-B/--cost). --- Maybe soon superseded by newer valuation code. +-- | Convert this journal's postings' amounts to cost using their transaction prices, +-- if specified by options (-B/--cost). Strip prices if not needed. journalSelectingAmountFromOpts :: ReportOpts -> Journal -> Journal -journalSelectingAmountFromOpts opts = case cost_ opts of +journalSelectingAmountFromOpts ropts = maybeStripPrices . case cost_ ropts of Cost -> journalToCost NoCost -> id + where + maybeStripPrices = if show_costs_ ropts then id else journalMapPostingAmounts mixedAmountStripPrices -- | Convert report options to a query, ignoring any non-flag command line arguments. queryFromFlags :: ReportOpts -> Query diff --git a/hledger-ui/Hledger/UI/TransactionScreen.hs b/hledger-ui/Hledger/UI/TransactionScreen.hs index 7303df1b8..b5319726f 100644 --- a/hledger-ui/Hledger/UI/TransactionScreen.hs +++ b/hledger-ui/Hledger/UI/TransactionScreen.hs @@ -83,7 +83,10 @@ tsDraw UIState{aopts=UIOpts{cliopts_=copts@CliOpts{reportspec_=rspec@ReportSpec{ render . defaultLayout toplabel bottomlabel . str . T.unpack . showTransactionOneLineAmounts - $ transactionApplyCostValuation prices styles periodlast (rsToday rspec) (cost_ ropts) (value_ ropts) t + . maybe id (transactionApplyValuation prices styles periodlast (rsToday rspec)) (value_ ropts) + $ case cost_ ropts of + Cost -> transactionToCost styles t + NoCost -> t -- (if real_ ropts then filterTransactionPostings (Real True) else id) -- filter postings by --real where toplabel =