From e05f20c135e3baf5ee92dccfc0bd810120db95c8 Mon Sep 17 00:00:00 2001 From: Bryce Date: Sat, 15 Aug 2026 16:24:39 -0700 Subject: [PATCH] fix(square): split per client, and say why same-client shares stay shared MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running the split across all 19,040,296 orders left 104 payments with two parent orders in a 250,000-order sample. Every one of them is two orders of the SAME client; cross-client sharing is gone entirely. Attempting to split those too was wrong twice over. Both orders compute the same name, so there is no second name to give a copy and the transaction conflicts. And a copy would double that client's takings for the day — where Square splits one tender across two of a client's own orders, one payment covering both is the truthful record. So the rule is now explicit: split per client, not per order. The component cascade still reaches these, which is why remove-voided-orders needs its own guard regardless of how complete this migration is — that was already the recommendation and this makes it load-bearing rather than belt-and-braces. Batch bookkeeping now records which name each charge was claimed under, so a second order in the same batch computing that same name is left alone instead of attempting a colliding copy. Migration state on the restore, measured rather than asserted: charges 17,046,418 scoped · 0 legacy · 0 unscopable refunds 51,986 scoped · 0 legacy payouts 144,652 scoped · 0 legacy · 36 with no owner shifts 69,291 scoped · 0 legacy A second full pass walked all 19M orders in 7.8 minutes and changed nothing, which is the idempotency the tests assert, confirmed at full scale. Co-Authored-By: Claude Opus 5 --- .../jobs/rekey_square_external_ids.clj | 24 ++++++++++++------ .../jobs/rekey_square_external_ids_test.clj | 25 +++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/clj/auto_ap/jobs/rekey_square_external_ids.clj b/src/clj/auto_ap/jobs/rekey_square_external_ids.clj index 7984b893..693fc55e 100644 --- a/src/clj/auto_ap/jobs/rekey_square_external_ids.clj +++ b/src/clj/auto_ap/jobs/rekey_square_external_ids.clj @@ -143,8 +143,15 @@ `:keep` — no other order has claimed this charge yet, so this order takes it and it is renamed in place. `:clone` — another order already owns it, so this order needs its own copy. - Whether a charge is already claimed is read from the charge itself: an unclaimed one still - carries the bare `square/charge/` form. Deciding it that way rather than by remembering + Splitting is per client, not per order. Where two orders of the SAME client and location refer + to one payment — Square splitting a tender across orders, or an amendment — they are left + sharing it deliberately. Both would compute the same name, so there is no second name to give + a copy, and more importantly a copy would double that client's takings for the day. The + component cascade still applies to those, which is why the retraction guard on + `remove-voided-orders` is needed regardless of this migration. + + Whether a charge is claimed is read from the charge itself: an unclaimed one still carries the + bare `square/charge/` form. Deciding it that way rather than by remembering every charge seen so far is what lets this run across all 16 million of them — the alternative needs a map of the entire table in memory. `batch-seen` covers only the orders inside one transaction, where the database snapshot cannot yet show a claim made moments earlier." @@ -159,9 +166,12 @@ :when old :let [raw (raw-square-id db charge old) new-key (str charge-prefix code "-" loc "-" raw) - unclaimed? (and (= old (str charge-prefix raw)) - (not (contains? @batch-seen charge)))] - :when (not= old new-key)] + claimed (get @batch-seen charge) + unclaimed? (and (= old (str charge-prefix raw)) (nil? claimed))] + ;; nothing to do when the charge already answers to this order's name, or when + ;; another order in this same batch has just claimed it under that very name — + ;; that is the same-client case, which stays shared + :when (and (not= old new-key) (not= claimed new-key))] {:order order-eid :charge charge :new-key new-key :raw raw :client (:db/id (:sales-order/client oe)) :location loc :action (if unclaimed? :keep :clone)})))) @@ -187,13 +197,13 @@ rekeyed (atom 0)] (doseq [batch (partition-all batch-size orders)] (let [db (dc/db conn) - batch-seen (atom #{}) + batch-seen (atom {}) tx (doall (for [o batch plan (charge-plan-for-order db o batch-seen) :let [{:keys [charge new-key action client location]} plan] tx-item (if (= :keep action) - (do (swap! batch-seen conj charge) + (do (swap! batch-seen assoc charge new-key) (swap! rekeyed inc) ;; record who claimed it: the owner is how a later order ;; recovers the Square id from an already-scoped key diff --git a/test/clj/auto_ap/jobs/rekey_square_external_ids_test.clj b/test/clj/auto_ap/jobs/rekey_square_external_ids_test.clj index 64595f13..a5825462 100644 --- a/test/clj/auto_ap/jobs/rekey_square_external_ids_test.clj +++ b/test/clj/auto_ap/jobs/rekey_square_external_ids_test.clj @@ -122,3 +122,28 @@ (is (not (clojure.string/includes? kb (str code-a "-CD"))) "the clone carries one scope, not two") (is (= 2 (charge-count))))))) + +(deftest two-orders-of-the-same-client-keep-sharing + (testing "one payment covering two of the SAME client's orders is left shared, on purpose. + + There is no second name to give a copy — both orders compute the same one — and a copy + would double that client's takings for the day. The component cascade still reaches + these, which is why remove-voided-orders needs its own guard." + (let [{:strs [test-client-id]} (setup-test-data []) + tx @(dc/transact conn [{:db/id "charge" + :charge/external-id "square/charge/same1" + :charge/total 75.0} + {:db/id "o1" :sales-order/external-id "square/order/x-1" + :sales-order/client test-client-id :sales-order/location "CD" + :sales-order/date sales-date :sales-order/charges ["charge"]} + {:db/id "o2" :sales-order/external-id "square/order/x-2" + :sales-order/client test-client-id :sales-order/location "CD" + :sales-order/date sales-date :sales-order/charges ["charge"]}]) + o1 (get-in tx [:tempids "o1"]) o2 (get-in tx [:tempids "o2"]) + code (:client/code (dc/entity (dc/db conn) test-client-id))] + (is (= {:rekeyed 1 :cloned 0} (sut/split-and-rekey-charges! [o1 o2] 100)) + "renamed once, not copied") + (is (= 1 (charge-count)) "no copy was made, so the takings are not doubled") + (is (= (str "square/charge/" code "-CD-same1") (:key (first (charges-of o1))))) + (is (= (:eid (first (charges-of o1))) (:eid (first (charges-of o2)))) + "both orders still point at the one payment"))))