From 4a7215a74b7316e42dce353a0c4795f2427a2c50 Mon Sep 17 00:00:00 2001 From: Bryce Covert Date: Thu, 9 Dec 2021 08:53:36 -0800 Subject: [PATCH] Fixes two types of bugs. --- ...-not-matching-invoices-or-transactions.clj | 35 ++-- src/clj/auto_ap/graphql/invoices.clj | 1 + src/clj/auto_ap/ledger.clj | 3 +- src/clj/auto_ap/rule_matching.clj | 41 +++-- src/clj/auto_ap/yodlee/import.clj | 150 +++++++++--------- .../clj/auto_ap/integration/rule_matching.clj | 99 ++++++++++++ .../clj/auto_ap/integration/yodlee/import.clj | 1 + 7 files changed, 225 insertions(+), 105 deletions(-) create mode 100644 test/clj/auto_ap/integration/rule_matching.clj diff --git a/scratch-sessions/find-ledgers-not-matching-invoices-or-transactions.clj b/scratch-sessions/find-ledgers-not-matching-invoices-or-transactions.clj index 190c547b..a18131d0 100644 --- a/scratch-sessions/find-ledgers-not-matching-invoices-or-transactions.clj +++ b/scratch-sessions/find-ledgers-not-matching-invoices-or-transactions.clj @@ -6,18 +6,27 @@ (require '[auto-ap.utils :refer [dollars=]]) (defn get-bad-ones [] - (->> (d/query {:query {:find ['?je '?a '(sum ?debit) '(sum ?credit)] - :with ['?jel] - :in '[$] - :where ['[?je :journal-entry/amount ?a] - '[?je :journal-entry/line-items ?jel] - '[(get-else $ ?jel :journal-entry-line/debit 0.0) ?debit] - '[(get-else $ ?jel :journal-entry-line/credit 0.0) ?credit]] - } - :args [(d/db auto-ap.datomic/conn)]}) - (filter (fn [[_ a d c]] - (or (not (dollars= a d)) - (not (dollars= a c))))) + + + (->> (->> (d/q '[:find ?c + :in $ + :where [?c :client/code]] + (d/db auto-ap.datomic/conn)) + (map first)) + (mapcat #(->> (d/query {:query {:find ['?je '?a '(sum ?debit) '(sum ?credit)] + :with ['?jel] + :in '[$ ?c] + :where ['[?je :journal-entry/client ?c] + '[?je :journal-entry/amount ?a] + '[?je :journal-entry/line-items ?jel] + '[(get-else $ ?jel :journal-entry-line/debit 0.0) ?debit] + '[(get-else $ ?jel :journal-entry-line/credit 0.0) ?credit]] + } + :args [(d/db auto-ap.datomic/conn) %]}) + (filter (fn [[_ a d c]] + (or (not (dollars= a d)) + (not (dollars= a c))))))) + #_(count) #_(take 3))) @@ -32,7 +41,7 @@ je)))) (filter (fn [invoice?] (:invoice/total invoice?))) - (map (fn [invoice] + #_(map (fn [invoice] {:total (:invoice/total invoice) :client-code (:client/code (:invoice/client invoice)) :invoice-number (:invoice/invoice-number invoice) diff --git a/src/clj/auto_ap/graphql/invoices.clj b/src/clj/auto_ap/graphql/invoices.clj index 922811d3..6c04aaa6 100644 --- a/src/clj/auto_ap/graphql/invoices.clj +++ b/src/clj/auto_ap/graphql/invoices.clj @@ -265,6 +265,7 @@ (assert-can-see-client (:id context) (:db/id (:invoice/client (d-invoices/get-by-id (:invoice_id args))))) (let [invoice-id (:invoice_id args) invoice (d-invoices/get-by-id invoice-id) + _ (assert-valid-expense-accounts (:expense_accounts args)) deleted (deleted-expense-accounts invoice (:expense_accounts args)) updated {:db/id invoice-id :invoice/expense-accounts (map diff --git a/src/clj/auto_ap/ledger.clj b/src/clj/auto_ap/ledger.clj index c1835437..e852eb77 100644 --- a/src/clj/auto_ap/ledger.clj +++ b/src/clj/auto_ap/ledger.clj @@ -338,7 +338,8 @@ :args [(d/db auto-ap.datomic/conn)]})) ] (filter - (fn [[e accounts]] (not= accounts (get jel-accounts e))) + (fn [[e accounts]] + (not= accounts (get jel-accounts e))) invoice-accounts))) (defn touch-broken-ledger [] diff --git a/src/clj/auto_ap/rule_matching.clj b/src/clj/auto_ap/rule_matching.clj index c4774cc7..6fe251fc 100644 --- a/src/clj/auto_ap/rule_matching.clj +++ b/src/clj/auto_ap/rule_matching.clj @@ -100,25 +100,36 @@ (map (fn [r] (update r :transaction-rule/description #(some-> % ->pattern)))) (filter #(rule-applies? transaction %)))) +(defn spread-cents [cents n] + (let [default-spread (for [x (range n)] + (int (* cents (/ 1.0 n)))) + short-by (- cents (reduce + 0 default-spread)) ;; amount that was lost in the differenc + adjusted-spread (map + (fn [cents increments] + (+ cents increments)) + + default-spread + (concat (take short-by (repeat 1)) (repeat 0)))] + (filter #(> % 0) adjusted-spread))) + (defn apply-rule [transaction rule valid-locations] (with-precision 2 (let [accounts (vec (mapcat (fn [tra] - (if (= "Shared" (:transaction-rule-account/location tra)) - (map - (fn [location] - {:transaction-account/account (:db/id (:transaction-rule-account/account tra)) - :transaction-account/amount (Math/abs (* (/ 1.0 (count valid-locations)) - (:transaction-rule-account/percentage tra) - (:transaction/amount transaction))) - :transaction-account/location location}) - - - valid-locations) - [(cond-> {:transaction-account/account (:db/id (:transaction-rule-account/account tra)) - :transaction-account/amount (Math/abs (* (:transaction-rule-account/percentage tra) - (:transaction/amount transaction)))} - (:transaction-rule-account/location tra) (assoc :transaction-account/location (:transaction-rule-account/location tra)))])) + (let [cents-to-distribute (int (Math/round (Math/abs (* (:transaction-rule-account/percentage tra) + (:transaction/amount transaction) + 100))))] + (if (= "Shared" (:transaction-rule-account/location tra)) + (->> valid-locations + (map + (fn [cents location] + {:transaction-account/account (:db/id (:transaction-rule-account/account tra)) + :transaction-account/amount (* 0.01 cents) + :transaction-account/location location}) + (spread-cents cents-to-distribute (count valid-locations)))) + [(cond-> {:transaction-account/account (:db/id (:transaction-rule-account/account tra)) + :transaction-account/amount (* 0.01 cents-to-distribute)} + (:transaction-rule-account/location tra) (assoc :transaction-account/location (:transaction-rule-account/location tra)))]))) (filter (comp seq :transaction-rule-account/account) (:transaction-rule/accounts rule)))) accounts (mapv (fn [a] diff --git a/src/clj/auto_ap/yodlee/import.clj b/src/clj/auto_ap/yodlee/import.clj index 5d296a71..56e768d5 100644 --- a/src/clj/auto_ap/yodlee/import.clj +++ b/src/clj/auto_ap/yodlee/import.clj @@ -173,86 +173,84 @@ first))) (defn transactions->txs [transactions transaction->bank-account apply-rules existing] - (doto - (into [] + (into [] - (for [transaction transactions - :let [{post-date :postDate - account-id :accountId - date :date - id :id - {amount :amount} :amount - {description-original :original - description-simple :simple} :description - {merchant-id :id - merchant-name :name} :merchant - base-type :baseType - type :type - status :status} - transaction - amount (if (= "DEBIT" base-type) - (- amount) - amount) - check-number (extract-check-number transaction) - bank-account (transaction->bank-account transaction) - bank-account-id (:db/id bank-account) - client (:client/_bank-accounts bank-account) - client-id (:db/id client) - valid-locations (or (:bank-account/locations bank-account) (:client/locations client)) - - date (time/parse date "YYYY-MM-dd")] - :when (and client-id - (not (existing (sha-256 (str id)))) - (= "POSTED" status) + (for [transaction transactions + :let [{post-date :postDate + account-id :accountId + date :date + id :id + {amount :amount} :amount + {description-original :original + description-simple :simple} :description + {merchant-id :id + merchant-name :name} :merchant + base-type :baseType + type :type + status :status} + transaction + amount (if (= "DEBIT" base-type) + (- amount) + amount) + check-number (extract-check-number transaction) + bank-account (transaction->bank-account transaction) + bank-account-id (:db/id bank-account) + client (:client/_bank-accounts bank-account) + client-id (:db/id client) + valid-locations (or (:bank-account/locations bank-account) (:client/locations client)) + + date (time/parse date "YYYY-MM-dd")] + :when (and client-id + (not (existing (sha-256 (str id)))) + (= "POSTED" status) - (or (not (:start-date bank-account)) - (t/after? date (:start-date bank-account))))] - (let [existing-check (transaction->existing-payment transaction check-number client-id bank-account-id amount id) - autopay-invoices-matches (when-not existing-check - (match-transaction-to-unfulfilled-autopayments amount client-id )) - unpaid-invoices-matches (when-not existing-check - (match-transaction-to-unpaid-invoices amount client-id )) - expected-deposit (when (and (> amount 0.0) - (not existing-check)) - (find-expected-deposit client-id amount date))] - (cond-> - [#:transaction - {:post-date (coerce/to-date (time/parse post-date "YYYY-MM-dd")) - :id (sha-256 (str id)) - :raw-id (str id) - :account-id account-id - :date (coerce/to-date date) - :amount (double amount) - :description-original (some-> description-original (str/replace #"\s+" " ")) - :description-simple (some-> description-simple (str/replace #"\s+" " ")) - :approval-status :transaction-approval-status/unapproved - :type type - :status status - :client client-id - :check-number check-number - :bank-account bank-account-id}] - existing-check (update 0 #(assoc % :transaction/approval-status :transaction-approval-status/approved - :transaction/payment {:db/id (:db/id existing-check) - :payment/status :payment-status/cleared} - :transaction/vendor (:db/id (:payment/vendor existing-check)) - :transaction/location "A" - :transaction/accounts [#:transaction-account - {:account (:db/id (a/get-account-by-numeric-code-and-sets 21000 ["default"])) - :location "A" - :amount (Math/abs (double amount))}])) + (or (not (:start-date bank-account)) + (t/after? date (:start-date bank-account))))] + (let [existing-check (transaction->existing-payment transaction check-number client-id bank-account-id amount id) + autopay-invoices-matches (when-not existing-check + (match-transaction-to-unfulfilled-autopayments amount client-id )) + unpaid-invoices-matches (when-not existing-check + (match-transaction-to-unpaid-invoices amount client-id )) + expected-deposit (when (and (> amount 0.0) + (not existing-check)) + (find-expected-deposit client-id amount date))] + (cond-> + [#:transaction + {:post-date (coerce/to-date (time/parse post-date "YYYY-MM-dd")) + :id (sha-256 (str id)) + :raw-id (str id) + :account-id account-id + :date (coerce/to-date date) + :amount (double amount) + :description-original (some-> description-original (str/replace #"\s+" " ")) + :description-simple (some-> description-simple (str/replace #"\s+" " ")) + :approval-status :transaction-approval-status/unapproved + :type type + :status status + :client client-id + :check-number check-number + :bank-account bank-account-id}] + existing-check (update 0 #(assoc % :transaction/approval-status :transaction-approval-status/approved + :transaction/payment {:db/id (:db/id existing-check) + :payment/status :payment-status/cleared} + :transaction/vendor (:db/id (:payment/vendor existing-check)) + :transaction/location "A" + :transaction/accounts [#:transaction-account + {:account (:db/id (a/get-account-by-numeric-code-and-sets 21000 ["default"])) + :location "A" + :amount (Math/abs (double amount))}])) - ;; temporarily removed to automatically match autopaid invoices - #_(and (not existing-check) - (seq autopay-invoices-matches)) #_(add-new-payment autopay-invoices-matches bank-account-id client-id) - expected-deposit (update 0 #(assoc % :transaction/expected-deposit {:db/id expected-deposit - :expected-deposit/status :expected-deposit-status/cleared})) - + ;; temporarily removed to automatically match autopaid invoices + #_(and (not existing-check) + (seq autopay-invoices-matches)) #_(add-new-payment autopay-invoices-matches bank-account-id client-id) + expected-deposit (update 0 #(assoc % :transaction/expected-deposit {:db/id expected-deposit + :expected-deposit/status :expected-deposit-status/cleared})) + - (and (not (seq autopay-invoices-matches)) - (not (seq unpaid-invoices-matches)) - (not expected-deposit)) (update 0 #(apply-rules % valid-locations)) - true (update 0 remove-nils))))) - println)) + (and (not (seq autopay-invoices-matches)) + (not (seq unpaid-invoices-matches)) + (not expected-deposit)) (update 0 #(apply-rules % valid-locations)) + true (update 0 remove-nils)))))) (defn batch-transact [transactions] diff --git a/test/clj/auto_ap/integration/rule_matching.clj b/test/clj/auto_ap/integration/rule_matching.clj new file mode 100644 index 00000000..f4c07a0b --- /dev/null +++ b/test/clj/auto_ap/integration/rule_matching.clj @@ -0,0 +1,99 @@ +(ns auto-ap.integration.rule-matching + (:require [auto-ap.rule-matching :as sut] + [datomic.api :as d] + [auto-ap.datomic :refer [uri]] + [auto-ap.datomic.migrate :as m] + [clojure.test :as t] + [clojure.tools.logging :as log] + [clojure.set :as set])) + +(defn wrap-setup + [f] + (with-redefs [auto-ap.datomic/uri "datomic:mem://datomic-transactor:4334/invoice"] + (d/create-database uri) + (with-redefs [auto-ap.datomic/conn (d/connect uri)] + (m/migrate auto-ap.datomic/conn) + (f) + (d/release auto-ap.datomic/conn) + (d/delete-database uri)))) + +(t/use-fixtures :each wrap-setup) + +(defn noop-rule [transaction locations] + transaction) + +(def base-transaction #:transaction {:amount -12.0 + :date #inst "2014-01-02T08:00:00.000-00:00" + :bank-account 456 + :client 123 + :post-date #inst "2014-01-04T08:00:00.000-00:00" + :account-id 1234 + :description-original "original-description" + :status "POSTED" + :id "6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b" + :raw-id "1" + :approval-status :transaction-approval-status/unapproved + :description-simple "simple-description"}) + + + +(t/deftest rule-applying-fn + (t/testing "Should apply if description matches" + (t/is (sut/rule-applies? + base-transaction + {:transaction-rule/description #"original-description" + :transaction-rule/transaction-approval-status :transaction-approval-status/approved})) + + (t/is (not (sut/rule-applies? + base-transaction + {:transaction-rule/description #"xxx" + :transaction-rule/transaction-approval-status :transaction-approval-status/approved})))) + + (t/testing "Should do nothing if there are no rules" + (let [process (sut/rule-applying-fn [])] + (t/is (= base-transaction + (process base-transaction ["NG"]))))) + + (t/testing "Should approve a simple rule" + (let [process (sut/rule-applying-fn [{:transaction-rule/description "simple-description" + :transaction-rule/transaction-approval-status :transaction-approval-status/approved}]) + transaction (assoc base-transaction :transaction/description-original "simple-description")] + (t/is (= :transaction-approval-status/approved + (:transaction/approval-status (process transaction ["NG"])))))) + + (t/testing "spread cents" + (t/testing "Should split evenly" + (t/is (= [50 50] (sut/spread-cents 100 2))) + (t/is (= [50 49] (sut/spread-cents 99 2))) + (t/is (= [34 33 33] (sut/spread-cents 100 3))) + (t/is (= [100] (sut/spread-cents 100 1))) + (t/is (= [2 2] (sut/spread-cents 4 2))) + (t/is (= [2 1 1] (sut/spread-cents 4 3))) + (t/is (= [2 2 1] (sut/spread-cents 5 3))) + (t/is (= [2 1 1 1] (sut/spread-cents 5 4))) + (t/is (= [3 2 2] (sut/spread-cents 7 3))) + (t/is (= [2 2 2 1] (sut/spread-cents 7 4))) + (t/is (= [2 1] (sut/spread-cents 3 2))) + (t/is (= [1 1 1] (sut/spread-cents 3 4))) + + (doseq [x (range 20)] + (t/is (= 11323 (reduce + 0 (sut/spread-cents 11323 (inc x)))))))) + + (t/testing "Should spread across locations" + (let [process (sut/rule-applying-fn [{:transaction-rule/description "simple-description" + :transaction-rule/accounts [{:transaction-rule-account/location "Shared" + :transaction-rule-account/account {:account/numeric-code 2500} + :transaction-rule-account/percentage 1.0}] + :transaction-rule/transaction-approval-status :transaction-approval-status/approved}]) + transaction (assoc base-transaction :transaction/description-original "simple-description" :transaction/amount 1.0)] + (t/is (= [1.0] + (map :transaction-account/amount (:transaction/accounts (process transaction ["NG"]))))) + (t/is (= [0.5 0.5] + (map :transaction-account/amount (:transaction/accounts (process transaction ["NG" "BT"]))))) + (t/is (= [0.34 0.33 0.33] + (map :transaction-account/amount (:transaction/accounts (process transaction ["NG" "BT" "DE"]))))) + (t/is (= [0.01 0.01] + (map :transaction-account/amount (:transaction/accounts (process (assoc transaction :transaction/amount 0.02) ["NG" "BT" "DE"]))))) + (t/is (= [0.02 0.01 0.01] + (map :transaction-account/amount (:transaction/accounts (process (assoc transaction :transaction/amount 0.04) ["NG" "BT" "DE"]))))) + ))) diff --git a/test/clj/auto_ap/integration/yodlee/import.clj b/test/clj/auto_ap/integration/yodlee/import.clj index 0161c913..fbb9edbe 100644 --- a/test/clj/auto_ap/integration/yodlee/import.clj +++ b/test/clj/auto_ap/integration/yodlee/import.clj @@ -55,6 +55,7 @@ :description-original "original-description" :status "POSTED" :id "6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b" + :raw-id "1" :approval-status :transaction-approval-status/unapproved :description-simple "simple-description"}] result))))