fix(square): do not double-scope a charge that is already scoped

The split derived the underlying Square id by stripping only the
"square/charge/" prefix. That is correct the first time a charge is seen, but
once it has been re-keyed to one client, a second order processing the same
charge in a later batch reads NGCC-CC-<id> as the id and scopes it again:

  square/charge/NGCD-CD-NGCC-CC-hW59pSj5hAsFBUIQi6cMyY36kN7YY

The importer then computes the correct single-scoped key, does not find it, and
creates a second charge — doubling the tender on exactly the days the import
touched. Found on a restored backup where five contended clients showed
imbalances of $3,000 to $7,000 on Aug 5-7.

Client codes may contain dashes, so the scope cannot be recognised by pattern.
raw-square-id recovers it from the entity instead: whoever the charge currently
belongs to is whose scope its key carries. The run also remembers each charge's
raw id when it first claims it, so later clones reuse it rather than re-deriving
from a mutated key.

Covered by a test that runs the split with a batch size of one, which is the
arrangement that made the second order observe an already-scoped key.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-15 13:32:50 -07:00
parent 784c221f99
commit 088dade112
2 changed files with 42 additions and 4 deletions

View File

@@ -115,6 +115,28 @@
[:charge/type-name :charge/total :charge/tip :charge/tax :charge/date
:charge/processor :charge/note :charge/reference-link])
(defn- raw-square-id
"The Square id inside a charge's external id, with any client scoping removed.
Stripping only the `square/charge/` prefix is not enough. Once a charge has been scoped to one
client, a second order processing the same charge would read `NGCC-CC-<id>` as the id and scope
it again, producing `square/charge/NGCD-CD-NGCC-CC-<id>`. The importer then computes the
correct single-scoped key, fails to find it, and creates a second charge — silently doubling
the tender.
Client codes may themselves contain dashes, so the scope cannot be recognised by pattern. It is
recovered from the entity instead: whoever the charge currently belongs to is exactly whose
scope its key carries."
[db charge old]
(let [ent (dc/entity db charge)
code (:client/code (:charge/client ent))
loc (:charge/location ent)
owner-prefix (when (and code loc) (str charge-prefix code "-" loc "-"))]
(cond
(and owner-prefix (.startsWith ^String old ^String owner-prefix)) (subs old (count owner-prefix))
(.startsWith ^String old charge-prefix) (subs old (count charge-prefix))
:else old)))
(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."
@@ -127,9 +149,9 @@
: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)
(let [raw (or (get @seen charge) (raw-square-id db charge old))
new-key (str charge-prefix code "-" loc "-" raw)]
{:order order-eid :charge charge :new-key new-key
{:order order-eid :charge charge :new-key new-key :raw raw
:client (:db/id (:sales-order/client oe)) :location loc
:action (if (contains? @seen charge) :clone :keep)})))))
@@ -150,7 +172,7 @@
`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 #{})
(let [seen (atom {})
cloned (atom 0)
rekeyed (atom 0)]
(doseq [batch (partition-all batch-size orders)]
@@ -160,7 +182,7 @@
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)
(do (swap! seen assoc charge (:raw plan))
(swap! rekeyed inc)
[{:db/id charge :charge/external-id new-key}])
(let [ent (dc/entity db charge)

View File

@@ -3,6 +3,7 @@
[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.string]
[clojure.test :refer [deftest is testing use-fixtures]]
[datomic.api :as dc]))
@@ -106,3 +107,18 @@
(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")))))
(deftest clone-is-not-double-scoped-across-batches
(testing "with a batch size of one, the second order sees a charge already carrying the first
client's scope. It must clone using the underlying Square id, not re-scope the scoped
key — otherwise the entity ends up keyed NGCD-CD-NGCC-CC-<id>, the importer computes
the correct key, misses, and creates a second charge that doubles the tender."
(let [{:keys [order-a order-b code-a code-b]} (two-orders-sharing-one-charge)]
(sut/split-and-rekey-charges! [order-a order-b] 1)
(let [ka (:key (first (charges-of order-a)))
kb (:key (first (charges-of order-b)))]
(is (= (str "square/charge/" code-a "-CD-shared1") ka))
(is (= (str "square/charge/" code-b "-CC-shared1") kb))
(is (not (clojure.string/includes? kb (str code-a "-CD")))
"the clone carries one scope, not two")
(is (= 2 (charge-count)))))))