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>
This commit was merged in pull request #19.
This commit is contained in:
@@ -132,7 +132,13 @@
|
|||||||
valid-clients]}
|
valid-clients]}
|
||||||
(cond-> {:query {:find []
|
(cond-> {:query {:find []
|
||||||
:in ['$ '[?clients ?start ?end]]
|
: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
|
:args [db
|
||||||
[valid-clients
|
[valid-clients
|
||||||
(some-> (:start-date query-params) coerce/to-date)
|
(some-> (:start-date query-params) coerce/to-date)
|
||||||
|
|||||||
68
test/clj/auto_ap/ssr/transaction/common_test.clj
Normal file
68
test/clj/auto_ap/ssr/transaction/common_test.clj
Normal 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))))))
|
||||||
Reference in New Issue
Block a user