From 58f6714399236671327d505b1cb164b4f3205700 Mon Sep 17 00:00:00 2001 From: Bryce Date: Sun, 16 Aug 2026 08:40:47 -0700 Subject: [PATCH] fix(transactions): drop suppressed transactions from the transactions page (#19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://gitea.story-basking.ts.net/notid/integreat/pulls/19 Co-authored-by: Bryce Co-committed-by: Bryce --- src/clj/auto_ap/ssr/transaction/common.clj | 8 ++- .../auto_ap/ssr/transaction/common_test.clj | 68 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 test/clj/auto_ap/ssr/transaction/common_test.clj diff --git a/src/clj/auto_ap/ssr/transaction/common.clj b/src/clj/auto_ap/ssr/transaction/common.clj index 20d8df71..39041111 100644 --- a/src/clj/auto_ap/ssr/transaction/common.clj +++ b/src/clj/auto_ap/ssr/transaction/common.clj @@ -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) diff --git a/test/clj/auto_ap/ssr/transaction/common_test.clj b/test/clj/auto_ap/ssr/transaction/common_test.clj new file mode 100644 index 00000000..be439d07 --- /dev/null +++ b/test/clj/auto_ap/ssr/transaction/common_test.clj @@ -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))))))