From 87f7fd564b043444998b411d6b7fa973f9a65f76 Mon Sep 17 00:00:00 2001 From: Bryce Date: Sat, 15 Aug 2026 12:01:43 -0700 Subject: [PATCH] fix(square): client-scope payout and cash-drawer-shift keys too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An audit of every unique-identity entity the importers create found two more that carry no client scoping: :expected-deposit/external-id "square/payout/" id :cash-drawer-shift/external-id "square/cash-drawer-shift/" id Both endpoints are location-filtered, so two clients configured on one Square location import the same payout or shift and collide on a single entity, exactly as refunds and charges did. This is not hypothetical. Reading ownership out of the history of the restored backup, the client has actually changed on: 4,069 of 144,688 expected deposits (2.8%) 2,628 of 69,291 cash drawer shifts (3.8%) 3,387 of 51,990 refunds (6.5%, before the earlier fix) across 19 distinct client pairs — and only 10 of those pairs share a location in today's configuration. Nine, including NGMJ/NGSC with 1,546 affected entities and NGAK/NGMH with 952, are invisible to any point-in-time check because the configuration has since changed. Deactivating what looks shared today is therefore a snapshot fix; scoping the keys is what makes contention structurally impossible. Verified on the restore: 144,652 deposits and 69,291 shifts re-keyed with all four entity counts unchanged, zero collisions, zero legacy keys remaining. 36 deposits carry neither client nor location and were left alone. ezCater orders and sales orders were already scoped and need no change. Co-Authored-By: Claude Opus 5 --- .../jobs/rekey_square_external_ids.clj | 27 ++++++++---- src/clj/auto_ap/square/core3.clj | 15 +++++-- test/clj/auto_ap/square/core3_test.clj | 41 +++++++++++++++++++ 3 files changed, 72 insertions(+), 11 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 dc508c89..349163c5 100644 --- a/src/clj/auto_ap/jobs/rekey_square_external_ids.clj +++ b/src/clj/auto_ap/jobs/rekey_square_external_ids.clj @@ -1,10 +1,15 @@ (ns auto-ap.jobs.rekey-square-external-ids - "One-shot migration re-keying Square refunds and charges to client-scoped external ids. + "One-shot migration re-keying Square entities to client-scoped external ids. - Refund and charge keys carry no client scoping today, so two clients configured on the same - Square location share a single entity: the refund's owner flips every time either client - imports, and one charge ends up referenced by both clients' orders. Sales orders already scope - their keys by client and location; this brings the other two in line. + Refunds, charges, Square payouts (expected deposits) and cash drawer shifts all carry keys with + no client scoping, so two clients configured on the same Square location share a single entity: + its owner flips every time either client imports. Sales orders and ezCater orders already scope + their keys by client and location; this brings the rest in line. + + Measured on a restored production backup, ownership had actually changed on 3,387 refunds, + 4,069 expected deposits and 2,628 cash drawer shifts, across 19 distinct client pairs — nine of + which no longer share a location in the current configuration and so are invisible to any + point-in-time check. Run AFTER the importer knows how to resolve both key schemes (`square.core3/existing-id`). Running it first would be harmless, but the importer would then re-create legacy-keyed @@ -19,6 +24,8 @@ (def refund-prefix "square/refund/") (def charge-prefix "square/charge/") +(def deposit-prefix "square/payout/") +(def shift-prefix "square/cash-drawer-shift/") (defn- scope-of "`[client-code location]` for an entity, or nil when it cannot be determined. @@ -35,7 +42,11 @@ :sales-refund/external-id (pair (:client/code (:sales-refund/client ent)) (:sales-refund/location ent)) :charge/external-id (pair (:client/code (:charge/client ent)) - (:charge/location ent))) + (:charge/location ent)) + :expected-deposit/external-id (pair (:client/code (:expected-deposit/client ent)) + (:expected-deposit/location ent)) + :cash-drawer-shift/external-id (pair (:client/code (:cash-drawer-shift/client ent)) + (:cash-drawer-shift/location ent))) (when-let [o (:e (first (dc/datoms db :vaet e :sales-order/charges)))] (let [oe (dc/entity db o)] (pair (:client/code (:sales-order/client oe)) (:sales-order/location oe)))) @@ -103,4 +114,6 @@ either number moves, the re-key created duplicates instead of updating in place." [db] {:refunds (reduce (fn [n _] (inc n)) 0 (dc/datoms db :aevt :sales-refund/external-id)) - :charges (reduce (fn [n _] (inc n)) 0 (dc/datoms db :aevt :charge/external-id))}) + :charges (reduce (fn [n _] (inc n)) 0 (dc/datoms db :aevt :charge/external-id)) + :deposits (reduce (fn [n _] (inc n)) 0 (dc/datoms db :aevt :expected-deposit/external-id)) + :shifts (reduce (fn [n _] (inc n)) 0 (dc/datoms db :aevt :cash-drawer-shift/external-id))}) diff --git a/src/clj/auto_ap/square/core3.clj b/src/clj/auto_ap/square/core3.clj index 43d118a0..fb695791 100644 --- a/src/clj/auto_ap/square/core3.clj +++ b/src/clj/auto_ap/square/core3.clj @@ -533,7 +533,7 @@ (try (->> (for [payout payouts :let [best-sales-date (some->> (dc/q '[:find ?s4 (count ?s) - :in $ ?payout-id + :in $ [?payout-id ...] :where [?payout :expected-deposit/external-id ?payout-id] [?payout :expected-deposit/charges ?c] @@ -543,7 +543,8 @@ [(auto-ap.time/localize ?s2) ?s3] [(clj-time.coerce/to-local-date ?s3) ?s4]] (dc/db conn) - (str "square/payout/" (:id payout))) + [(scoped-key "square/payout/" client location (:id payout)) + (str "square/payout/" (:id payout))]) (sort-by last) last first @@ -567,7 +568,10 @@ (:db/id client) (amount->money (:amount_money payout))))] :when (not equivalent-already-exists?)] - #:expected-deposit {:external-id (str "square/payout/" (:id payout)) + #:expected-deposit {:db/id (or (existing-id (dc/db conn) :expected-deposit/external-id + "square/payout/" client location (:id payout)) + (str "square/payout/" (:id payout))) + :external-id (scoped-key "square/payout/" client location (:id payout)) :vendor :vendor/ccp-square :status :expected-deposit-status/pending :total (amount->money (:amount_money payout)) @@ -732,7 +736,10 @@ (de/chain (get-cash-shift client (:id s)) (fn [cash-drawer-shift] - #:cash-drawer-shift {:external-id (str "square/cash-drawer-shift/" (:id cash-drawer-shift)) + #:cash-drawer-shift {:db/id (or (existing-id (dc/db conn) :cash-drawer-shift/external-id + "square/cash-drawer-shift/" client l (:id cash-drawer-shift)) + (str "square/cash-drawer-shift/" (:id cash-drawer-shift))) + :external-id (scoped-key "square/cash-drawer-shift/" client l (:id cash-drawer-shift)) :vendor :vendor/ccp-square :paid-in (amount->money (:cash_paid_in_money cash-drawer-shift)) :paid-out (amount->money (:cash_paid_out_money cash-drawer-shift)) diff --git a/test/clj/auto_ap/square/core3_test.clj b/test/clj/auto_ap/square/core3_test.clj index 0f642c91..b99eeeef 100644 --- a/test/clj/auto_ap/square/core3_test.clj +++ b/test/clj/auto_ap/square/core3_test.clj @@ -75,3 +75,44 @@ :sales-refund/external-id (sut/scoped-key "square/refund/" other other-loc "shared") :sales-refund/total 10.0}]) (is (= 2 (refund-count)) "two stable entities, one per client, rather than one that flips")))) + +(deftest payouts-and-shifts-are-client-scoped-too + (testing "expected deposits and cash drawer shifts are fetched per location, so two clients on + one location collide on them exactly as refunds and charges did" + (is (= "square/payout/NGCD-CD-po1" + (sut/scoped-key "square/payout/" client location "po1"))) + (is (= "square/cash-drawer-shift/NGCD-CD-sh1" + (sut/scoped-key "square/cash-drawer-shift/" client location "sh1"))))) + +(deftest legacy-keyed-deposit-is-updated-not-duplicated + (testing "a payout still carrying its unscoped key is found and re-keyed in place" + (setup-test-data []) + @(dc/transact conn [{:db/id "d" + :expected-deposit/external-id "square/payout/po1" + :expected-deposit/total 100.0}]) + (let [eid (sut/existing-id (dc/db conn) :expected-deposit/external-id "square/payout/" client location "po1")] + (is (some? eid) "resolves the entity carrying the legacy key") + @(dc/transact conn [{:db/id eid + :expected-deposit/external-id (sut/scoped-key "square/payout/" client location "po1") + :expected-deposit/total 100.0}]) + (is (= 1 (count (dc/q '[:find ?e :where [?e :expected-deposit/external-id]] (dc/db conn)))) + "no second deposit was created") + (is (nil? (dc/entid (dc/db conn) [:expected-deposit/external-id "square/payout/po1"])) + "the legacy key is gone")))) + +(deftest two-clients-get-their-own-deposit + (testing "with the client in the key, a second client importing the same Square payout creates + its own entity instead of taking ownership of the first" + (setup-test-data []) + (let [other {:client/code "NGCC"} + other-loc {:square-location/client-location "CC"}] + @(dc/transact conn [{:db/id "d" + :expected-deposit/external-id (sut/scoped-key "square/payout/" client location "shared") + :expected-deposit/total 100.0}]) + (is (nil? (sut/existing-id (dc/db conn) :expected-deposit/external-id "square/payout/" other other-loc "shared")) + "the second client does not resolve onto the first client's deposit") + @(dc/transact conn [{:db/id "d2" + :expected-deposit/external-id (sut/scoped-key "square/payout/" other other-loc "shared") + :expected-deposit/total 100.0}]) + (is (= 2 (count (dc/q '[:find ?e :where [?e :expected-deposit/external-id]] (dc/db conn)))) + "two stable entities, one per client, rather than one that flips"))))