fix(square): split shared charges instead of re-keying them to one client
Re-keying alone could not undo an existing shared charge. It handed the single entity to whichever client was looked at first and left the other order pointing at a charge it does not own. Because :sales-order/charges is a component attribute, that is not untidy but dangerous: retracting either order deletes a charge the other still needs. It also produced double tender when the second client re-imported and created its own. split-and-rekey-charges! now gives every order its own charge. The first order to claim a shared charge keeps it, re-keyed to that order's client and location; every other order gets a copy carrying the same amounts, scoped to itself, with its reference repointed. Afterwards no charge has more than one parent order and the component relationship means what it says. charges-with-multiple-parents is the §3.3 gate, which must read zero before any historical cleanup or voided-order retraction is safe. Four tests cover it: that the shared condition exists to begin with, that the split produces two distinct entities with amounts copied and one parent each, that an unshared charge is only re-keyed, and that re-running changes nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -109,6 +109,88 @@
|
||||
(alog/info ::migrated :attr attr :done (* i batch-size) :of total)))
|
||||
total))
|
||||
|
||||
(def charge-copy-attrs
|
||||
"Everything a charge carries in its own right. `:charge/client+date` is a tuple Datomic
|
||||
maintains, and `:charge/external-id` is set separately, so neither is copied."
|
||||
[:charge/type-name :charge/total :charge/tip :charge/tax :charge/date
|
||||
:charge/processor :charge/note :charge/reference-link])
|
||||
|
||||
(defn- charge-plan-for-order
|
||||
"What one order needs doing to its charges: `:keep` where the charge is already this order's
|
||||
alone, `:clone` where it is shared with another order and this order needs its own copy."
|
||||
[db order-eid seen]
|
||||
(let [oe (dc/entity db order-eid)
|
||||
code (:client/code (:sales-order/client oe))
|
||||
loc (:sales-order/location oe)]
|
||||
(when (and code loc)
|
||||
(for [d (dc/datoms db :eavt order-eid :sales-order/charges)
|
||||
:let [charge (:v d)
|
||||
old (:v (first (dc/datoms db :eavt charge :charge/external-id)))]
|
||||
:when old]
|
||||
(let [raw (if (.startsWith ^String old charge-prefix) (subs old (count charge-prefix)) old)
|
||||
new-key (str charge-prefix code "-" loc "-" raw)]
|
||||
{:order order-eid :charge charge :new-key new-key
|
||||
:client (:db/id (:sales-order/client oe)) :location loc
|
||||
:action (if (contains? @seen charge) :clone :keep)})))))
|
||||
|
||||
(defn split-and-rekey-charges!
|
||||
"Gives every order its own charge entity, keyed by that order's client and location.
|
||||
|
||||
Where two clients were configured on one Square location, both clients' orders resolved to a
|
||||
single charge, because charge keys carried no client. Re-keying alone does not undo that — it
|
||||
hands the one entity to whichever client is looked at first and leaves the other order pointing
|
||||
at a charge it does not own. Since `:sales-order/charges` is a component attribute, that is not
|
||||
merely untidy: retracting either order would delete a charge the other one still needs.
|
||||
|
||||
So a shared charge is cloned. The first order to claim it keeps it, re-keyed to that order's
|
||||
scope; every other order gets a copy carrying the same amounts, scoped to its own client, and
|
||||
has its reference repointed. Afterwards no charge has more than one parent order and the
|
||||
component relationship means what it says.
|
||||
|
||||
`orders` is the collection of order entity ids to process — typically every order belonging to
|
||||
the clients that share, or have ever shared, a Square location."
|
||||
[orders batch-size]
|
||||
(let [seen (atom #{})
|
||||
cloned (atom 0)
|
||||
rekeyed (atom 0)]
|
||||
(doseq [batch (partition-all batch-size orders)]
|
||||
(let [db (dc/db conn)
|
||||
tx (doall
|
||||
(for [o batch
|
||||
plan (charge-plan-for-order db o seen)
|
||||
:let [{:keys [charge new-key action client location]} plan]
|
||||
tx-item (if (= :keep action)
|
||||
(do (swap! seen conj charge)
|
||||
(swap! rekeyed inc)
|
||||
[{:db/id charge :charge/external-id new-key}])
|
||||
(let [ent (dc/entity db charge)
|
||||
copy (reduce (fn [m a] (if-some [v (get ent a)]
|
||||
(assoc m a (if (map? v) (:db/id v) v))
|
||||
m))
|
||||
{} charge-copy-attrs)]
|
||||
(swap! cloned inc)
|
||||
[(assoc copy
|
||||
:db/id new-key
|
||||
:charge/external-id new-key
|
||||
:charge/client client
|
||||
:charge/location location)
|
||||
[:db/retract o :sales-order/charges charge]
|
||||
{:db/id o :sales-order/charges new-key}]))]
|
||||
tx-item))]
|
||||
(when (seq tx) @(dc/transact conn tx))))
|
||||
(alog/info ::split-charges :rekeyed @rekeyed :cloned @cloned)
|
||||
{:rekeyed @rekeyed :cloned @cloned}))
|
||||
|
||||
(defn charges-with-multiple-parents
|
||||
"The §3.3 gate. Must read zero once the split has run: while any charge has two parent orders,
|
||||
retracting either order deletes the other one's payment."
|
||||
[db orders]
|
||||
(->> orders
|
||||
(mapcat (fn [o] (map :v (dc/datoms db :eavt o :sales-order/charges))))
|
||||
distinct
|
||||
(filter (fn [c] (> (reduce (fn [n _] (inc n)) 0 (dc/datoms db :vaet c :sales-order/charges)) 1)))
|
||||
count))
|
||||
|
||||
(defn counts
|
||||
"Entity totals, for the before/after assertion that is this migration's real safety net: if
|
||||
either number moves, the re-key created duplicates instead of updating in place."
|
||||
|
||||
108
test/clj/auto_ap/jobs/rekey_square_external_ids_test.clj
Normal file
108
test/clj/auto_ap/jobs/rekey_square_external_ids_test.clj
Normal file
@@ -0,0 +1,108 @@
|
||||
(ns auto-ap.jobs.rekey-square-external-ids-test
|
||||
(:require
|
||||
[auto-ap.datomic :refer [conn]]
|
||||
[auto-ap.integration.util :refer [setup-test-data wrap-setup]]
|
||||
[auto-ap.jobs.rekey-square-external-ids :as sut]
|
||||
[clojure.test :refer [deftest is testing use-fixtures]]
|
||||
[datomic.api :as dc]))
|
||||
|
||||
(use-fixtures :each wrap-setup)
|
||||
|
||||
(def sales-date #inst "2026-08-01T07:00:00.000-00:00")
|
||||
|
||||
(defn- charge-count []
|
||||
(count (dc/q '[:find ?e :where [?e :charge/external-id]] (dc/db conn))))
|
||||
|
||||
(defn- charges-of [order]
|
||||
(->> (dc/datoms (dc/db conn) :eavt order :sales-order/charges)
|
||||
(map :v)
|
||||
(map (fn [c] {:eid c
|
||||
:key (:v (first (dc/datoms (dc/db conn) :eavt c :charge/external-id)))
|
||||
:total (:v (first (dc/datoms (dc/db conn) :eavt c :charge/total)))
|
||||
:tip (:v (first (dc/datoms (dc/db conn) :eavt c :charge/tip)))}))
|
||||
vec))
|
||||
|
||||
(defn- parents-of [charge]
|
||||
(reduce (fn [n _] (inc n)) 0 (dc/datoms (dc/db conn) :vaet charge :sales-order/charges)))
|
||||
|
||||
(defn- two-orders-sharing-one-charge []
|
||||
(let [{:strs [test-client-id]} (setup-test-data [])
|
||||
other (get-in @(dc/transact conn [{:db/id "other" :client/code "NGCC"}]) [:tempids "other"])
|
||||
tx @(dc/transact conn [{:db/id "charge"
|
||||
:charge/external-id "square/charge/shared1"
|
||||
:charge/type-name "CARD"
|
||||
:charge/total 120.0
|
||||
:charge/tip 20.0}
|
||||
{:db/id "order-a"
|
||||
:sales-order/external-id "square/order/NGCD-CD-o1"
|
||||
:sales-order/client test-client-id
|
||||
:sales-order/location "CD"
|
||||
:sales-order/date sales-date
|
||||
:sales-order/charges ["charge"]}
|
||||
{:db/id "order-b"
|
||||
:sales-order/external-id "square/order/NGCC-CC-o1"
|
||||
:sales-order/client other
|
||||
:sales-order/location "CC"
|
||||
:sales-order/date sales-date
|
||||
:sales-order/charges ["charge"]}])]
|
||||
{:order-a (get-in tx [:tempids "order-a"])
|
||||
:order-b (get-in tx [:tempids "order-b"])
|
||||
:charge (get-in tx [:tempids "charge"])
|
||||
:code-a (:client/code (dc/entity (dc/db conn) test-client-id))
|
||||
:code-b "NGCC"}))
|
||||
|
||||
(deftest a-shared-charge-starts-with-two-parents
|
||||
(testing "the condition under test really exists before the split runs"
|
||||
(let [{:keys [charge]} (two-orders-sharing-one-charge)]
|
||||
(is (= 1 (charge-count)))
|
||||
(is (= 2 (parents-of charge))
|
||||
"one charge entity, referenced by both clients' orders"))))
|
||||
|
||||
(deftest split-gives-each-order-its-own-charge
|
||||
(testing "each order ends up with its own charge, scoped to its own client, carrying the same
|
||||
amounts — so the component relationship means what it says and retracting one order
|
||||
cannot delete the other's payment"
|
||||
(let [{:keys [order-a order-b code-a code-b]} (two-orders-sharing-one-charge)
|
||||
result (sut/split-and-rekey-charges! [order-a order-b] 100)]
|
||||
(is (= {:rekeyed 1 :cloned 1} result) "first order keeps it, second gets a copy")
|
||||
(is (= 2 (charge-count)) "exactly one new entity was created")
|
||||
|
||||
(let [a (charges-of order-a)
|
||||
b (charges-of order-b)]
|
||||
(is (= 1 (count a)))
|
||||
(is (= 1 (count b)))
|
||||
(is (= (str "square/charge/" code-a "-CD-shared1") (:key (first a))))
|
||||
(is (= (str "square/charge/" code-b "-CC-shared1") (:key (first b))))
|
||||
(is (not= (:eid (first a)) (:eid (first b))) "two distinct entities")
|
||||
(is (= 120.0 (:total (first a)) (:total (first b))) "amounts copied")
|
||||
(is (= 20.0 (:tip (first a)) (:tip (first b))) "tips copied")
|
||||
(is (= 1 (parents-of (:eid (first a)))))
|
||||
(is (= 1 (parents-of (:eid (first b))))
|
||||
"no charge has more than one parent order any more")))))
|
||||
|
||||
(deftest split-leaves-an-unshared-charge-alone
|
||||
(testing "an order that already owns its charge outright is only re-keyed, never cloned"
|
||||
(let [{:strs [test-client-id]} (setup-test-data [])
|
||||
tx @(dc/transact conn [{:db/id "charge"
|
||||
:charge/external-id "square/charge/solo1"
|
||||
:charge/total 50.0}
|
||||
{:db/id "order"
|
||||
:sales-order/external-id "square/order/NGCD-CD-o2"
|
||||
:sales-order/client test-client-id
|
||||
:sales-order/location "CD"
|
||||
:sales-order/date sales-date
|
||||
:sales-order/charges ["charge"]}])
|
||||
order (get-in tx [:tempids "order"])
|
||||
code (:client/code (dc/entity (dc/db conn) test-client-id))]
|
||||
(is (= {:rekeyed 1 :cloned 0} (sut/split-and-rekey-charges! [order] 100)))
|
||||
(is (= 1 (charge-count)) "nothing was created")
|
||||
(is (= (str "square/charge/" code "-CD-solo1") (:key (first (charges-of order))))))))
|
||||
|
||||
(deftest split-is-idempotent
|
||||
(testing "re-running over an already-split database changes nothing further"
|
||||
(let [{:keys [order-a order-b]} (two-orders-sharing-one-charge)]
|
||||
(sut/split-and-rekey-charges! [order-a order-b] 100)
|
||||
(let [after-first (charge-count)]
|
||||
(is (= {:rekeyed 2 :cloned 0} (sut/split-and-rekey-charges! [order-a order-b] 100))
|
||||
"each order now owns its charge outright, so both are simple re-keys")
|
||||
(is (= after-first (charge-count)) "and no further entities appear")))))
|
||||
Reference in New Issue
Block a user