2 Commits

Author SHA1 Message Date
69eae3b7dc Merge branch 'staging' of gitea.story-basking.ts.net:notid/integreat into staging 2026-08-16 08:41:06 -07:00
58f6714399 fix(transactions): drop suppressed transactions from the transactions page (#19)
Suppressed transactions were showing up on the transactions page.

## The cause

The SSR transactions page never excluded `:transaction-approval-status/suppressed`. The GraphQL/CLJS transactions page has always excluded them unconditionally (`auto_ap/datomic/transactions.clj`, the `true` branch of its `cond->>`), as do the ledger queries in `auto_ap/ledger.clj`.

The `scan-transactions` ion does no status filtering of its own — it is a bare `index-range` over `:transaction/client+date` — so nothing downstream was dropping them either. Suppressing a transaction is the user's way of saying "stop showing me this", and it stayed visible on the page it was suppressed from.

## Impact

22,813 suppressed transactions exist db-wide out of 621,492. For the worst-affected client, a five-year range returned **14,736 rows against 3,641 real ones** — the other 11,095 were suppressed.

## The fix

One `not` clause in the main scan in `auto_ap/ssr/transaction/common.clj`, applied inside the query rather than after it so the row count and the amount total — both derived from `fetch-ids` — match the rows actually rendered. Measured 6ms -> 29ms on that worst-case client and range.

The three status routes (approved, unapproved, requires-feedback) constrain the status themselves and there is no suppressed route, so no page loses its own rows.

## Note on the commit list

This branch carries `6e7f66a7` (include-in-reports in the register) and its revert. That commit was only ever on a local staging; it never reached this remote. The net diff of this branch against `staging` is exactly the transactions fix below — the add and the revert cancel out. Both are kept so local staging reconciles cleanly after merge.

## Testing

- New `test/clj/auto_ap/ssr/transaction/common_test.clj` — 6 assertions, passing. Covers the suppressed row being dropped from `ids`, `all-ids` and `count`; the approved status route still listing its rows; and an all-suppressed client returning empty.
- `auto-ap.ssr.transaction.import-test` and `auto-ap.ssr.ledger-test` pass, with one exception: `external-import-remove-button-test` fails identically with and without these changes, so it is pre-existing and unrelated.
- `auto-ap.ssr.transaction.edit-test` still has its known `apply-rule does not exist` compile break, untouched here.
- `lein cljfmt check` clean.

## Loose end, not addressed

`:transaction/exclude-from-ledger` is defined in `resources/schema.edn` but read nowhere in the codebase — only `:invoice/exclude-from-ledger` is honored. If anything ever wrote that attribute expecting it to hide transactions, it never did.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Reviewed-on: #19
Co-authored-by: Bryce <bryce@brycecovertoperations.com>
Co-committed-by: Bryce <bryce@brycecovertoperations.com>
2026-08-16 08:40:47 -07:00
4 changed files with 191 additions and 1 deletions

View File

@@ -203,6 +203,39 @@
result))
results))
(defn- entries-off-reports
"Journal entries with a line posted to one of `clients`' bank accounts flagged
`:bank-account/include-in-reports` false, or nil when there are none.
The GraphQL ledger page has always dropped these (see
auto-ap.datomic.ledger/graphql-results), so the register has to agree --
otherwise the same entry is visible on one ledger page and missing from the
other. Resolved up front off VAET, which is ~150 entries db-wide and costs
~2ms; excluding them inside the query with a not-join instead measured ~10x
the cost of the whole scan on a wide date range."
[db clients]
(when-let [off-reports (seq (dc/q '[:find [?ba ...]
:in $ [?client ...]
:where
[?client :client/bank-accounts ?ba]
[?ba :bank-account/include-in-reports false]]
db clients))]
(set (dc/q '[:find [?e ...]
:in $ [?ba ...]
:where
[?li :journal-entry-line/account ?ba]
[?e :journal-entry/line-items ?li]]
db off-reports))))
(defn- apply-include-in-reports [db clients results]
(if-let [hidden (entries-off-reports db clients)]
(remove (comp hidden last) results)
results))
;; TODO
;; 1. Sorting in investigate dialog
;; 2. actual date range filtering in investigate dialog
@@ -375,6 +408,7 @@
(merge-query {:query {:find ['?sort-default '?e]}})))]
(->> (observable-query query)
(apply-include-in-reports db valid-clients)
(apply-sort-4 (assoc query-params :default-asc? true))
(apply-only-unbalanced query-params)
(apply-pagination query-params))))

View File

@@ -132,7 +132,13 @@
valid-clients]}
(cond-> {:query {:find []
:in ['$ '[?clients ?start ?end]]
:where '[[(iol-ion.query/scan-transactions $ ?clients ?start ?end) [[?e _ ?sort-default] ...]]]}
;; Suppressed transactions are never listed -- the GraphQL page has
;; always dropped them (auto-ap.datomic.transactions/graphql-results),
;; as do the ledger queries. The three status routes (approved,
;; unapproved, requires-feedback) constrain the status themselves, and
;; there is no suppressed route, so this never hides a page's own rows.
:where '[[(iol-ion.query/scan-transactions $ ?clients ?start ?end) [[?e _ ?sort-default] ...]]
(not [?e :transaction/approval-status :transaction-approval-status/suppressed])]}
:args [db
[valid-clients
(some-> (:start-date query-params) coerce/to-date)

View File

@@ -797,3 +797,85 @@
[:journal-entry/external-id (external-id "ext-good-1")])]
(is (= 100.0 (:journal-entry/amount entry)))
(is (= 2 (count (:journal-entry/line-items entry))))))))))))
(deftest fetch-ids-include-in-reports-test
(testing "Should leave an entry out of the register when a line posts to a bank account off reports"
(let [{:strs [reports-client shown-entry]}
(setup-test-data [(test-bank-account :db/id "shown-bank"
:bank-account/name "Shown Bank"
:bank-account/numeric-code 11000
:bank-account/include-in-reports true)
(test-bank-account :db/id "hidden-bank"
:bank-account/name "Hidden Bank"
:bank-account/numeric-code 11001
:bank-account/include-in-reports false)
(test-client :db/id "reports-client"
:client/code "REPORTS-TEST"
:client/locations ["HQ"]
:client/bank-accounts ["shown-bank" "hidden-bank"])
{:db/id "reports-expense"
:account/name "Expense"
:account/numeric-code 60000
:account/account-set "default"}
{:db/id "shown-entry"
:journal-entry/client "reports-client"
:journal-entry/date #inst "2024-03-01"
:journal-entry/amount 100.0
:journal-entry/source "transaction"
:journal-entry/line-items
[{:journal-entry-line/account "shown-bank"
:journal-entry-line/credit 100.0}
{:journal-entry-line/account "reports-expense"
:journal-entry-line/debit 100.0}]}
{:db/id "hidden-entry"
:journal-entry/client "reports-client"
:journal-entry/date #inst "2024-03-02"
:journal-entry/amount 50.0
:journal-entry/source "transaction"
:journal-entry/line-items
[{:journal-entry-line/account "hidden-bank"
:journal-entry-line/credit 50.0}
{:journal-entry-line/account "reports-expense"
:journal-entry-line/debit 50.0}]}])
request {:client-id reports-client
:clients [reports-client]
:route-params {}
:query-params {}}
result (common/fetch-ids (dc/db conn) request)]
(is (= [shown-entry] (vec (:all-ids result))))
(is (= [shown-entry] (vec (:ids result))))
(testing "and counts it out of the total, so the page is not silently short"
(is (= 1 (:count result))))))
(testing "Should keep every entry when no bank account is off reports"
(let [{:strs [open-client open-entry]}
(setup-test-data [(test-bank-account :db/id "open-bank"
:bank-account/name "Open Bank"
:bank-account/numeric-code 11002
:bank-account/include-in-reports true)
(test-client :db/id "open-client"
:client/code "REPORTS-OPEN"
:client/locations ["HQ"]
:client/bank-accounts ["open-bank"])
{:db/id "open-expense"
:account/name "Expense"
:account/numeric-code 60001
:account/account-set "default"}
{:db/id "open-entry"
:journal-entry/client "open-client"
:journal-entry/date #inst "2024-03-01"
:journal-entry/amount 100.0
:journal-entry/source "transaction"
:journal-entry/line-items
[{:journal-entry-line/account "open-bank"
:journal-entry-line/credit 100.0}
{:journal-entry-line/account "open-expense"
:journal-entry-line/debit 100.0}]}])
result (common/fetch-ids (dc/db conn) {:client-id open-client
:clients [open-client]
:route-params {}
:query-params {}})]
(is (= [open-entry] (vec (:all-ids result))))
(is (= 1 (:count result))))))

View File

@@ -0,0 +1,68 @@
(ns auto-ap.ssr.transaction.common-test
(:require
[auto-ap.datomic :refer [conn]]
[auto-ap.integration.util :refer [setup-test-data test-bank-account test-client
test-transaction wrap-setup]]
[auto-ap.ssr.transaction.common :as sut]
[clojure.test :refer [deftest is testing use-fixtures]]
[datomic.api :as dc]))
(use-fixtures :each wrap-setup)
(defn- fetch
"Runs the transactions page query the way the page does, over a range wide
enough to cover the fixture data."
[client-id & {:keys [route-params query-params]}]
(sut/fetch-ids (dc/db conn)
{:clients [client-id]
:route-params (or route-params {})
:query-params (merge {:start-date "2021-01-01"
:end-date "2023-12-31"}
query-params)}))
(deftest fetch-ids-suppressed-test
(let [{:strs [suppressed-client kept-tx suppressed-tx]}
(setup-test-data
[(test-bank-account :db/id "suppressed-bank")
(test-client :db/id "suppressed-client"
:client/bank-accounts ["suppressed-bank"])
(test-transaction :db/id "kept-tx"
:transaction/client "suppressed-client"
:transaction/bank-account "suppressed-bank"
:transaction/date #inst "2022-06-01"
:transaction/amount 100.0
:transaction/approval-status :transaction-approval-status/approved)
(test-transaction :db/id "suppressed-tx"
:transaction/client "suppressed-client"
:transaction/bank-account "suppressed-bank"
:transaction/date #inst "2022-06-02"
:transaction/amount 250.0
:transaction/approval-status :transaction-approval-status/suppressed)])]
(testing "Should leave a suppressed transaction out of the transactions page"
(let [{:keys [ids all-ids count]} (fetch suppressed-client)]
(is (= [kept-tx] (vec ids)))
(is (not (contains? (set all-ids) suppressed-tx))
"a suppressed transaction must not reach the amount total either")
(is (= 1 count)
"the row count has to match what the page renders, not the unfiltered scan")))
(testing "Should still list a transaction on an explicit status route"
(let [{:keys [ids]} (fetch suppressed-client
:route-params {:status :transaction-approval-status/approved})]
(is (= [kept-tx] (vec ids)))))
(testing "Should return nothing for a client whose transactions are all suppressed"
(let [{:strs [empty-client]}
(setup-test-data
[(test-bank-account :db/id "empty-bank")
(test-client :db/id "empty-client"
:client/bank-accounts ["empty-bank"])
(test-transaction :db/id "only-tx"
:transaction/client "empty-client"
:transaction/bank-account "empty-bank"
:transaction/date #inst "2022-06-03"
:transaction/approval-status :transaction-approval-status/suppressed)])
{:keys [ids count]} (fetch empty-client)]
(is (empty? ids))
(is (= 0 count))))))