feat(square): client-scope refund and charge external ids

Sales order keys already carry client and location; refund and charge keys do
not. That is why two clients configured on the same Square location share one
entity: the refund's owner flips every time either client imports, and a single
charge ends up referenced by both clients' orders. Scoping the keys the same way
makes contention structurally impossible — each client gets its own entity.

The hazard is the cutover. These ids are :db.unique/identity and the import
relies on upsert-by-identity, so changing the key format alone would match
nothing and Datomic would create a SECOND entity for every refund and charge,
orphaning the originals under their legacy keys. square.core3/existing-id
resolves the entity explicitly, scoped key first and legacy key second, and pins
the result as :db/id so the write lands on the existing entity whichever scheme
it currently carries.

All three construction sites are covered: order tenders, refunds, and the payout
path, which mints bare charge stubs from an external id alone.

The migration job re-keys whatever the importer has not yet touched. It recovers
scope from the referencing sales order or expected deposit for the ~12.8% of
charges that carry neither :charge/client nor :charge/location, detects
already-scoped entities by comparing against the key they should have rather
than pattern-matching ids that may themselves contain dashes, and is therefore
idempotent and re-runnable over a partially migrated database.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-15 07:49:31 -07:00
parent b4b68b36cf
commit 42a8207be9
3 changed files with 215 additions and 3 deletions

View File

@@ -0,0 +1,106 @@
(ns auto-ap.jobs.rekey-square-external-ids
"One-shot migration re-keying Square refunds and charges 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.
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
entities on its next pass.
The migration is idempotent: an entity already carrying its scoped key is skipped, so it can
be re-run over a partially migrated database."
(:require
[auto-ap.datomic :refer [conn]]
[auto-ap.logging :as alog]
[datomic.api :as dc]))
(def refund-prefix "square/refund/")
(def charge-prefix "square/charge/")
(defn- scope-of
"`[client-code location]` for an entity, or nil when it cannot be determined.
Charges are the awkward case: about an eighth of them carry neither `:charge/client` nor
`:charge/location`. Those are stubs minted by the payout path, which asserts an external id
alone and lets unique-identity upsert bring a bare entity into being, plus older tender
records that predate the client attribute. None are orphaned, so the scope is recovered from
whatever references them — the sales order first, then the expected deposit."
[db attr e]
(let [ent (dc/entity db e)
pair (fn [code loc] (when (and code loc) [code loc]))]
(or (case attr
: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)))
(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))))
(when-let [d (:e (first (dc/datoms db :vaet e :expected-deposit/charges)))]
(let [de (dc/entity db d)]
(pair (:client/code (:expected-deposit/client de)) (:expected-deposit/location de)))))))
(defn planned-key
"`[eid new-key]` for an entity that still needs re-keying, or nil when it is already scoped or
cannot be scoped at all.
Detection compares against the key this entity *should* have rather than pattern-matching the
id, because Square ids may themselves contain dashes and no pattern separates the two schemes
reliably. That also makes the migration idempotent."
[db attr prefix datom]
(let [old (:v datom)]
(when-let [[code loc] (scope-of db attr (:e datom))]
(let [scoped-prefix (str prefix code "-" loc "-")]
(when-not (.startsWith ^String old scoped-prefix)
[(:e datom) (str scoped-prefix (subs old (count prefix)))])))))
(defn plan
"Everything the migration would change, plus what it cannot touch. Read-only — run this and
check `:collisions` is empty before transacting anything."
[db attr prefix]
(let [acc (reduce (fn [acc d]
(let [acc (update acc :total inc)]
(if-let [[e new-key] (planned-key db attr prefix d)]
(-> acc
(update :to-migrate inc)
(update :new-keys conj! [e new-key]))
(if (scope-of db attr (:e d))
(update acc :already-scoped inc)
(update acc :unscopable inc)))))
{:total 0 :to-migrate 0 :already-scoped 0 :unscopable 0 :new-keys (transient [])}
(dc/datoms db :aevt attr))]
(update acc :new-keys persistent!)))
(defn collisions
"Any two entities that would land on the same new key. Must be empty: a collision would merge
two entities into one and lose whichever lost."
[new-keys]
(->> new-keys
(group-by second)
(keep (fn [[k es]] (when (> (count es) 1) [k (mapv first es)])))
vec))
(defn migrate!
"Asserts the new external id on each planned entity. The attribute is cardinality one, so the
legacy value is retracted by the same assertion and the entity keeps its identity — nothing is
created and nothing is deleted.
Returns the number of entities re-keyed."
[attr new-keys batch-size]
(let [total (count new-keys)]
(alog/info ::migrating :attr attr :count total)
(doseq [[i batch] (map-indexed vector (partition-all batch-size new-keys))]
@(dc/transact conn (for [[e new-key] batch] {:db/id e attr new-key}))
(when (zero? (mod i 20))
(alog/info ::migrated :attr attr :done (* i batch-size) :of total)))
total))
(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."
[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))})

View File

@@ -269,6 +269,29 @@
0.0
[]))
(defn scoped-key
"Client-scoped external id, in the shape sales order keys already use.
Without the client and location in the key, two clients configured on the same Square location
collide on a single entity: a refund changes owner every time either client imports, and one
charge ends up shared between both clients' orders."
[prefix client location id]
(str prefix (:client/code client) "-" (:square-location/client-location location) "-" id))
(defn existing-id
"Entity id of the refund or charge this id already refers to, trying the client-scoped key
first and the legacy unscoped key second.
This is what makes re-keying safe. These external ids are `:db.unique/identity`, so the import
relies on upsert-by-identity; changing the key format on its own would match nothing and
Datomic would create a SECOND entity for every refund and charge, orphaning the original under
its legacy key. Pinning the result as `:db/id` makes the write land on the existing entity
whichever scheme it currently carries."
[db attr prefix client location id]
(when id
(or (dc/entid db [attr (scoped-key prefix client location id)])
(dc/entid db [attr (str prefix id)]))))
(defn tender->charge [order client location t]
(remove-nils
#:charge
@@ -278,8 +301,9 @@
:note (:note t)
:location (:square-location/client-location location)
:reference-link (str (url/url "https://squareup.com/receipt/preview" (:id t)))
:db/id (existing-id (dc/db conn) :charge/external-id "square/charge/" client location (:id t))
:external-id (when (:id t)
(str "square/charge/" (:id t)))
(scoped-key "square/charge/" client location (:id t)))
:processor (cond
(#{"OTHER" "THIRD_PARTY_CARD"} (:type t))
(condp = (some-> (:note t) str/lower-case)
@@ -561,7 +585,11 @@
(coerce/to-date)))
:charges (reverse (->> (:payout_entries payout)
(filter (comp :payment_id :type_charge_details))
(map (fn [p] {:charge/external-id (str "square/charge/" (:payment_id (:type_charge_details p)))}))))})
(map (fn [p]
(let [payment-id (:payment_id (:type_charge_details p))]
(remove-nils
{:charge/external-id (scoped-key "square/charge/" client location payment-id)
:db/id (existing-id (dc/db conn) :charge/external-id "square/charge/" client location payment-id)}))))))})
(filter :expected-deposit/date)
(into []))
(catch Throwable e
@@ -585,7 +613,8 @@
(de/chain
(get-payment client (:payment_id r))
(fn [payment]
#:sales-refund {:external-id (str "square/refund/" (:id r))
#:sales-refund {:db/id (existing-id (dc/db conn) :sales-refund/external-id "square/refund/" client l (:id r))
:external-id (scoped-key "square/refund/" client l (:id r))
:vendor :vendor/ccp-square
:total (amount->money (:amount_money r))
:fee (transduce

View File

@@ -0,0 +1,77 @@
(ns auto-ap.square.core3-test
(:require
[auto-ap.datomic :refer [conn]]
[auto-ap.integration.util :refer [setup-test-data wrap-setup]]
[auto-ap.square.core3 :as sut]
[clojure.test :refer [deftest is testing use-fixtures]]
[datomic.api :as dc]))
(use-fixtures :each wrap-setup)
(def client {:client/code "NGCD"})
(def location {:square-location/client-location "CD"})
(defn- refund-count []
(count (dc/q '[:find ?e :where [?e :sales-refund/external-id]] (dc/db conn))))
(defn- resolve-refund [id]
(sut/existing-id (dc/db conn) :sales-refund/external-id "square/refund/" client location id))
(deftest scoped-key-carries-client-and-location
(testing "the same shape sales order keys already use, so a shared location cannot contend"
(is (= "square/refund/NGCD-CD-abc" (sut/scoped-key "square/refund/" client location "abc")))
(is (= "square/charge/NGCD-CD-xyz" (sut/scoped-key "square/charge/" client location "xyz")))))
(deftest legacy-keyed-entity-is-updated-not-duplicated
(testing "an entity still carrying its unscoped key is found and re-keyed in place.
This is the sharpest hazard in the migration: these external ids are
:db.unique/identity, so writing the new key without resolving the old one first
matches nothing and creates a second entity, orphaning the original."
(setup-test-data [])
@(dc/transact conn [{:db/id "r"
:sales-refund/external-id "square/refund/abc"
:sales-refund/total 10.0}])
(is (= 1 (refund-count)))
(let [eid (resolve-refund "abc")]
(is (some? eid) "resolves an entity carrying the legacy key")
@(dc/transact conn [{:db/id eid
:sales-refund/external-id (sut/scoped-key "square/refund/" client location "abc")
:sales-refund/total 10.0}])
(is (= 1 (refund-count)) "no second entity was created")
(is (= eid (dc/entid (dc/db conn) [:sales-refund/external-id "square/refund/NGCD-CD-abc"]))
"the same entity now answers to the scoped key")
(is (nil? (dc/entid (dc/db conn) [:sales-refund/external-id "square/refund/abc"]))
"and no longer to the legacy one"))))
(deftest already-scoped-entity-resolves-by-its-new-key
(testing "re-running the importer after migration finds the entity by the scoped key, so the
migration is not undone and nothing is duplicated"
(setup-test-data [])
@(dc/transact conn [{:db/id "r"
:sales-refund/external-id "square/refund/NGCD-CD-abc"
:sales-refund/total 10.0}])
(is (= (dc/entid (dc/db conn) [:sales-refund/external-id "square/refund/NGCD-CD-abc"])
(resolve-refund "abc")))
(is (= 1 (refund-count)))))
(deftest unknown-id-resolves-to-nothing
(testing "a refund never seen before has no id to pin, so the importer creates it fresh"
(setup-test-data [])
(is (nil? (resolve-refund "never-seen")))))
(deftest two-clients-on-one-location-get-their-own-entities
(testing "the point of the re-key: with the client in the key, a second client importing the
same Square refund 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 "r"
:sales-refund/external-id (sut/scoped-key "square/refund/" client location "shared")
:sales-refund/total 10.0}])
(is (nil? (sut/existing-id (dc/db conn) :sales-refund/external-id "square/refund/" other other-loc "shared"))
"the second client does not resolve onto the first client's entity")
@(dc/transact conn [{:db/id "r2"
: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"))))