notid/integreat
1
0
Fork 0
Code Issues Pull Requests 4 Actions Packages Projects Releases Wiki Activity

fix(reports): collapse accounts sharing a numeric code to one row

Browse Source 
A client's bank account and the financial account it posts to carry the
same numeric code. Reports keyed their detail rows on [code, name], so
the pair rendered as two rows — one labelled for the bank account, one
for the financial account.

The amount was duplicated too, not just the label: the row's figure is
filtered by code alone, so both rows printed the whole code's total. The
group subtotal counts the code once, so a section's visible rows stopped
footing to their own subtotal. Penelope's Coffee and Tea overshot the
2000 Accounts Payable subtotal by $251,751.80 this way.

Resolve it in two places:

- build-account-lookup now maps every account at a shared code down to
  the bank account's name, per client. Where a client has two bank
  accounts on one code, lowest :bank-account/sort-order wins, then
  lowest :db/id, so the label is stable across runs.
- used-accounts now keys rows on the code alone. Across a multi-client
  report the clients can still disagree, since only some of them have a
  bank account at the code; a bank-sourced name wins there, which the
  new :bank_account_name? flag carries through from the lookup.

Rows are code-keyed now, so detail-rows decides whether to print a
figure by asking whether the client has data at the code rather than
under the winning name — otherwise a client reaching a code under a name
another client won would blank out.

Balance sheet, profit and loss and cash flows all route through
used-accounts and are all fixed. The GraphQL and cljs balance sheets
pick up the unified name through build-account-lookup.

A sweep of all 146 clients with bank accounts finds duplicate rows on 8
of them before this change and none after, with every section total
unchanged.
This commit is contained in:
Bryce
2026-08-14 13:55:38 -07:00
parent 366781e818
commit e9970bd41a
7 changed files with 298 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

129
test/clj/auto_ap/ledger/reports_test.clj Normal file
View File

@@ -0,0 +1,129 @@
(ns auto-ap.ledger.reports-test
(:require
[auto-ap.ledger.reports :as sut]
[clojure.test :refer [deftest is testing]]))
;; A client's bank account and the financial account it posts to share a
;; numeric code. Reports key their rows off that code, so before the fix the
;; pair rendered as two rows carrying the same code-level total, and the
;; section stopped footing to its own subtotal.
(def ^:private period #inst "2026-08-14")
(defn- account
[numeric-code name amount]
{:client-id 1
:location "M"
:numeric-code numeric-code
:name name
:amount amount
:debits 0.0
:credits amount
:count 1
:account-type :account-type/liability
:period period})
(defn- bank-account
"Same as `account`, but its name came from a :bank-account rather than the
chart of accounts — which is what earns it the row label."
[numeric-code name amount]
(assoc (account numeric-code name amount) :bank-account-name? true))
(defn- pnl-data
[data]
(sut/->PNLData {:periods [period]} data {1 "CLIENT"}))
(defn- labels
[rows]
(map (comp :value first) rows))
(deftest used-accounts-collapses-a-shared-numeric-code
(testing "two names on one code yield a single entry, named for the bank account"
(is (= [{:numeric-code 21010 :name "Capital One CC - 3196"}]
(sut/used-accounts
[{:data [(bank-account 21010 "Capital One CC - 3196" -100.0)
(account 21010 "Accounts Payable 10" -150.0)]}]))))
(testing "the bank account wins even when the financial name is more common"
(is (= [{:numeric-code 21010 :name "Capital One CC - 3196"}]
(sut/used-accounts
[{:data [(bank-account 21010 "Capital One CC - 3196" -100.0)]}
{:data [(account 21010 "Accounts Payable 10" -150.0)]}
{:data [(account 21010 "Accounts Payable 10" -150.0)]}]))))
(testing "distinct codes are left alone, ordered by code"
(is (= [{:numeric-code 21009 :name "Due to Grand Ventures"}
{:numeric-code 21010 :name "Capital One CC - 3196"}]
(sut/used-accounts
[{:data [(bank-account 21010 "Capital One CC - 3196" -100.0)
(account 21009 "Due to Grand Ventures" 25.0)]}]))))
(testing "with no bank account in play the most common name wins"
(is (= [{:numeric-code 21010 :name "Accounts Payable 10"}]
(sut/used-accounts
[{:data [(account 21010 "Due to Sandwich Monkey" -100.0)]}
{:data [(account 21010 "Accounts Payable 10" -150.0)]}
{:data [(account 21010 "Accounts Payable 10" -150.0)]}]))))
(testing "an even split breaks alphabetically so runs are reproducible"
(is (= [{:numeric-code 21010 :name "Accounts Payable 10"}]
(sut/used-accounts
[{:data [(account 21010 "Due to Sandwich Monkey" -100.0)]}
{:data [(account 21010 "Accounts Payable 10" -150.0)]}])))
(is (= [{:numeric-code 21010 :name "Amex - 41001"}]
(sut/used-accounts
[{:data [(bank-account 21010 "BofA CC - 8779" -100.0)]}
{:data [(bank-account 21010 "Amex - 41001" -150.0)]}]))
"two bank accounts still resolve to one, deterministically")))
(deftest balance-sheet-renders-one-row-per-shared-code
(let [report (sut/summarize-balance-sheet
(pnl-data [(bank-account 21010 "Capital One CC - 3196" -100.0)
(account 21010 "Accounts Payable 10" -150.0)
(account 21009 "Due to Grand Ventures" 25.0)]))
rows (:rows report)]
(testing "the shared code appears once, under the bank account's name"
(is (= 1 (count (filter #(= "Capital One CC - 3196:21010" %) (labels rows)))))
(is (not (some #{"Accounts Payable 10:21010"} (labels rows)))))
(testing "its row carries the total for the code, not one account's share"
(is (= -250.0
(->> rows
(filter #(= "Capital One CC - 3196:21010" (:value (first %))))
first
second
:value))))
(testing "the detail rows foot to the section subtotal"
(let [detail (->> rows
(filter #(re-find #":\d+$" (str (:value (first %)))))
(map (comp :value second)))
subtotal (->> rows
(filter #(= "Liabilities" (:value (first %))))
(keep (comp :value second))
last)]
(is (= 2 (count detail))
"three accounts across two codes collapse to two rows")
(is (= -225.0 (reduce + 0.0 detail) subtotal)
"before the fix 21010 printed twice and the rows overshot the subtotal")))))
(deftest balance-sheet-keeps-a-clients-figure-when-another-client-named-the-code
(testing "a column is blank only when that client has nothing at the code"
(let [rows (:rows (sut/summarize-balance-sheet
(sut/->PNLData
{:periods [period]}
[(assoc (bank-account 21010 "Capital One CC - 3196" -100.0) :client-id 1)
(assoc (account 21010 "Accounts Payable 10" -150.0) :client-id 2)
(assoc (account 21009 "Due to Grand Ventures" 25.0) :client-id 1)]
{1 "ONE" 2 "TWO"})))
shared (->> rows
(filter #(re-find #":21010$" (str (:value (first %)))))
first)
absent (->> rows
(filter #(re-find #":21009$" (str (:value (first %)))))
first)]
(is (= [-100.0 -150.0] (map :value (drop-last (rest shared))))
"client TWO reaches 21010 under a name client ONE won, and still shows its balance")
(is (= [25.0 ""] (map :value (drop-last (rest absent))))
"client TWO has nothing at 21009 and stays blank"))))

92
test/clj/auto_ap/ledger_test.clj
View File

@@ -1,10 +1,98 @@
(ns auto-ap.ledger-test
(:require
[auto-ap.integration.util :refer [wrap-setup]]
[clojure.test :as t]))
[auto-ap.datomic :refer [conn]]
[auto-ap.integration.util :refer [test-account test-bank-account test-client
wrap-setup]]
[auto-ap.ledger :refer [build-account-lookup]]
[clojure.test :as t]
[datomic.api :as dc]))
(t/use-fixtures :each wrap-setup)
(t/deftest build-account-lookup-prefers-the-bank-account-name
(t/testing "a bank account and the financial account sharing its code resolve to one name"
(let [{:strs [client bank financial]}
(:tempids @(dc/transact conn
[(test-account :db/id "financial"
:account/name "Accounts Payable 10"
:account/numeric-code 21010
:account/type :account-type/liability)
(test-client :db/id "client"
:client/bank-accounts
[(test-bank-account :db/id "bank"
:bank-account/name "Capital One CC - 3196"
:bank-account/numeric-code 21010
:bank-account/type :bank-account-type/credit)])]))
lookup (build-account-lookup client)]
(t/is (= "Capital One CC - 3196" (:name (lookup bank))))
(t/is (= "Capital One CC - 3196" (:name (lookup financial)))
"the financial account takes the bank account's name, so reports render one row")
(t/testing "and both are flagged as bank-named for downstream report labelling"
(t/is (true? (:bank_account_name? (lookup bank))))
(t/is (true? (:bank_account_name? (lookup financial)))))
(t/testing "without changing the code or the account type"
(t/is (= 21010 (:numeric_code (lookup bank))))
(t/is (= 21010 (:numeric_code (lookup financial))))
(t/is (= :account-type/liability (:account_type (lookup financial))))))))
(t/deftest build-account-lookup-leaves-unshared-codes-alone
(t/testing "an account with no bank account at its code keeps its own name"
(let [{:strs [client financial]}
(:tempids @(dc/transact conn
[(test-account :db/id "financial"
:account/name "Sales Taxes Payable"
:account/numeric-code 23000
:account/type :account-type/liability)
(test-client :db/id "client"
:client/bank-accounts
[(test-bank-account :db/id "bank"
:bank-account/name "Capital One CC - 3196"
:bank-account/numeric-code 21010
:bank-account/type :bank-account-type/credit)])]))
lookup (build-account-lookup client)]
(t/is (= "Sales Taxes Payable" (:name (lookup financial))))
(t/is (false? (:bank_account_name? (lookup financial))))))
(t/testing "another client's bank account never renames this client's accounts"
(let [{:strs [mine financial]}
(:tempids @(dc/transact conn
[(test-account :db/id "financial"
:account/name "Accounts Payable 10"
:account/numeric-code 21010
:account/type :account-type/liability)
(test-client :db/id "mine")
(test-client :db/id "theirs"
:client/bank-accounts
[(test-bank-account :db/id "bank"
:bank-account/name "Capital One CC - 3196"
:bank-account/numeric-code 21010
:bank-account/type :bank-account-type/credit)])]))
lookup (build-account-lookup mine)]
(t/is (= "Accounts Payable 10" (:name (lookup financial))))
(t/is (false? (:bank_account_name? (lookup financial)))))))
(t/deftest build-account-lookup-breaks-ties-between-bank-accounts
(t/testing "two bank accounts on one code resolve deterministically by sort-order"
(let [{:strs [client first-bank second-bank]}
(:tempids @(dc/transact conn
[(test-client :db/id "client"
:client/bank-accounts
[(test-bank-account :db/id "second-bank"
:bank-account/name "US Bank 2974"
:bank-account/numeric-code 13101
:bank-account/sort-order 3)
(test-bank-account :db/id "first-bank"
:bank-account/name "Fremont Gyro HB Main 8576"
:bank-account/numeric-code 13101
:bank-account/sort-order 0)])]))
lookup (build-account-lookup client)]
(t/is (= "Fremont Gyro HB Main 8576"
(:name (lookup first-bank))
(:name (lookup second-bank)))))))
(t/deftest entity-change->ledger
#_(t/testing "Should code an expected deposit"
(let [{:strs [ed ccp receipts-split client]}