This is transitional. Once no legacy names remain, the fallback and the refusal are deleted
+ together and the guarantee stops depending on either.
@@ -568,15 +579,20 @@
The problem this work exists to solve is gone: no payment answers to two orders, so the
component relationship means what it says and deleting an order can no longer take another
order's money with it.
-
One behaviour changed when the pass was run over everything, and it is worth
- recording. Where Square splits one tender across two of a single client's own orders,
- the earlier design left the payment shared on purpose — both orders compute the same name, so
- there is no second name for a copy to take. That rule only holds for two orders processed in
- the same batch. Run across nineteen million orders in batches of two thousand, such pairs
- almost always fall in different batches and the second order now takes a copy. Inside the
- ninety-day window this changed nothing measurable: the recompute after the database-wide pass
- matched the one before it to the cent. Outside the window it has not been measured, and it
- should be before this runs against production.
+
Where Square splits one tender across two of a single client's own orders, the
+ payment stays shared — at any batch size. Both orders compute the same name, so there
+ is no second name for a copy to take, and a copy would double that client's takings for the
+ day. The mechanism is worth stating precisely, because it is not obvious from reading: once
+ the first order re-keys the payment it also writes the owner attributes in the same
+ transaction, so a later order recovers the bare Square id from those, computes the name the
+ payment already carries, and the guard (not= old new-key) drops the
+ row before any copy decision is reached. Verified by running the migration at a batch size of
+ one, which forces the two orders into separate batches: no copy is made.
+
An earlier draft of this report claimed the opposite — that such pairs would be
+ copied once the batches split them — and flagged it as unmeasured risk to check before
+ production. That was wrong, and it is recorded here rather than quietly deleted because it did
+ real damage: an independent reviewer cited this paragraph as evidence and raised a defect that
+ does not exist. A test now pins the behaviour at batch size one.
The guard on remove-voided-orders is still worth having regardless. It is
cheap, and it makes the safety a property of the deletion rather than of the migration having
been run first.
diff --git a/docs/2026-08-15-sales-summary-rollout-plan.md b/docs/2026-08-15-sales-summary-rollout-plan.md
index 9a720aea..a7a3dd71 100644
--- a/docs/2026-08-15-sales-summary-rollout-plan.md
+++ b/docs/2026-08-15-sales-summary-rollout-plan.md
@@ -9,8 +9,8 @@ Measured on a restored copy of production (backup point `209608347`), 210 client
with zero days knocked out of balance and zero already-balanced days altered.
Of the 279 days left, **171 are not balancing faults** — they are days where a client's sales were
-never imported while its refunds were. Step 9 is about those, and it is the most important thing in
-this document.
+never imported while its refunds were. Step 10 is about those, and it is the most important
+thing in this document.
---
@@ -23,16 +23,54 @@ this document.
| Expected migration runtime | ~13 minutes for 19M orders on a warm cache |
| Nothing here touches | invoices, payments, the ledger, or any client without the flag set |
-**One prerequisite that is not code.** Ten Square locations are configured against two client records
-each. Someone in the business has to decide which record survives at each. The newer record usually
-has no history from before the split, so keeping it loses years of that location's books. Do this
-before step 3.
+**Client configuration is left exactly as it is.** Ten Square locations are configured against two
+client records each, and both stay active. The re-key is what resolves them: once every record
+carries its owner in its key, each client's import resolves only its own records and the two
+records keep independent, stable histories. No "which record survives" decision is needed, and
+nothing is deactivated.
+
+The consequence to be aware of: each Square payment, refund, payout and shift at a shared location
+becomes **two entities, one per client record** — by design. That is the stable end state, not a
+duplicate to clean up. If any report or export aggregates across client records, one restaurant's
+takings would be counted twice at that layer. Nothing in this work changes that either way.
+
+**The only window of risk is between deploying and finishing the migration**, while legacy keys
+still exist for a client to resolve. Steps 2–5 exist to make that window effectively zero.
---
-## Step 1 — Deploy the code
+## Step 1 — Guard `remove-voided-orders`
-Deploy the branch as normal. The flag is absent from every client, so:
+Do this before the migration, not after. `:sales-order/charges` is `:db/isComponent true`, so
+retracting an order cascades into its payments. Until step 4 finishes there are still payments with
+two parent orders, and deleting one client's voided order can take the other client's payment with
+it.
+
+Either leave `remove-voided-orders` switched off until step 4 verifies clean, or change it to detach
+a payment that has more than one parent rather than delete it. Detaching is worth doing regardless —
+it makes the safety a property of the deletion rather than of the migration having been run first.
+
+See `docs/2026-08-15-remove-voided-orders-risk.md`.
+
+---
+
+## Step 2 — Pause the Square importer
+
+**This is what makes the deploy safe, and it is easy to skip.** Steps 2 through 5 should be one
+maintenance action, not separate days' work.
+
+While legacy keys exist, `square.core3/existing-id` falls back to them — and at a shared location
+that is the one code path that can reach across client records. Running the migration with imports
+paused means no client is resolving keys while the keys are being rewritten, so the window closes
+entirely rather than merely narrowing.
+
+The migration itself takes about **13 minutes** for all 19M orders, so the pause is short.
+
+---
+
+## Step 3 — Deploy the code
+
+Deploy the branch. The flag is absent from every client, so:
- tips are calculated exactly as they are today,
- no `Service Charges` line is written.
@@ -41,60 +79,33 @@ The only changes that take effect immediately are the safe ones: imbalance loggi
dirty-summary scan bounded to one client (1,321 ms → 5.6 ms per client), the schema-ordering fix,
and the importer's new client-scoped keys.
-**The importer starts writing client-scoped keys straight away, and reads both schemes.** That is
-deliberate and is what makes the deploy independent of the migration. Do not remove the legacy
-lookup in `square.core3/existing-id` yet — see step 8.
-
-**Verify before moving on.** After one nightly import cycle:
-
-```clojure
-;; refunds, payouts and shifts must not have doubled
-(count (d/datoms (d/db conn) :aevt :sales-refund/external-id))
-(count (d/datoms (d/db conn) :aevt :expected-deposit/external-id))
-(count (d/datoms (d/db conn) :aevt :cash-drawer-shift/external-id))
-```
-
-Compare against the same counts taken immediately before deploy. Growth should be ordinary daily
-volume. A near-doubling means the legacy fallback is not working — **stop and roll back the deploy**.
-
----
-
-## Step 2 — Guard `remove-voided-orders`
-
-Do this before the migration, not after. `:sales-order/charges` is `:db/isComponent true`, so
-retracting an order cascades into its payments. Until step 3 finishes there are still payments with
-two parent orders, and deleting one client's voided order can take the other client's payment with
-it.
-
-Either leave `remove-voided-orders` switched off until step 3 completes, or change it to detach a
-payment that has more than one parent rather than delete it. Detaching is worth doing regardless —
-it makes the safety a property of the deletion rather than of the migration having been run first.
-
-See `docs/2026-08-15-remove-voided-orders-risk.md`.
-
----
-
-## Step 3 — Retire the duplicate client records
-
-Business decision from the top of this document. Deactivate the losing record's Square location so
-the importer stops fetching for it. The record itself stays; its history is untouched.
-
-**Verify:** no Square location is configured against two active client records.
+**The importer reads both key schemes**, so the deploy does not depend on the migration having
+finished. Two protections cover the interval before it does: imports are paused (step 2), and
+`existing-id` refuses to resolve a record that already belongs to a different client. Do not remove
+the legacy lookup yet — see step 9.
---
## Step 4 — Run the migration
+Run it immediately after the deploy, while imports are still paused.
+
```clojure
(require '[auto-ap.jobs.rekey-square-external-ids :as rk])
-;; read-only first — check :collisions is empty for every attribute
-(dissoc (rk/plan (d/db conn) :charge/external-id rk/charge-prefix) :new-keys)
+;; read-only first — no two entities may want the same key. `plan` does NOT return a
+;; :collisions key; you have to hand its :new-keys to `collisions` yourself.
+(rk/collisions (:new-keys (rk/plan (d/db conn) :charge/external-id rk/charge-prefix)))
+;; => [] (anything else: stop, do not migrate)
;; then the whole thing
(rk/migrate-all! 2000)
```
+`migrate-all!` runs this same check itself, on every attribute including charges, and throws
+rather than transacting if it finds one. Running it by hand first just means finding out before
+the 13-minute walk rather than partway through it.
+
Runs in about thirteen minutes over 19M orders. It is **idempotent and resumable** — a record that
already carries the right name is skipped, so it can be stopped and re-run without consequence.
@@ -102,25 +113,69 @@ If it appears to crawl, the cause is almost certainly garbage collection in the
not the transactor. That misdiagnosis cost two days of projected runtime during this work. Free
retained memory in the REPL and re-measure before changing anything about the database.
-**Verify — all four must read zero to migrate and zero unscopable:**
+**Verify.** Two checks, doing two different jobs — run both.
+
+**(a) Completeness, across everything.** `plan` must report nothing left to do, for all four
+attributes:
```clojure
-(rk/unscoped-report (d/db conn))
(dissoc (rk/plan (d/db conn) :charge/external-id rk/charge-prefix) :new-keys)
;; => {:total 17047142 :to-migrate 0 :already-scoped 17047142 :unscopable 0}
+```
-;; and the gate that this work exists for
-(rk/charges-with-multiple-parents (d/db conn) (take 400000 (rk/all-order-ids (d/db conn))))
+Read `:to-migrate 0` **and** `:unscopable 0`. This is the authoritative signal, and it covers all
+17M charges.
+
+`unscoped-report` is useful colour but is not the gate: its `:no-owner` column never reaches zero
+for charges, because ~283k payout stubs carry no `:charge/client` of their own and it classifies
+by attribute rather than by resolving ownership. Judge completeness by `plan`.
+
+**(b) The safety gate for the cascade** — no payment may answer to two orders, or re-enabling
+`remove-voided-orders` in step 8 can delete a payment another order still needs. Check **every**
+order in the last year, with no sampling:
+
+```clojure
+(let [db (d/db conn)
+ cs (map first (d/q '[:find ?c :where [?c :client/code _]] db))
+ year (java.util.Date. (- (.getTime (java.util.Date.)) (long (* 365 86400000))))]
+ (rk/charges-with-multiple-parents
+ db (map first (iol-ion.query/scan-sales-orders db cs year nil))))
;; => 0
```
-Note `unscoped-report`'s `:no-owner` column is not a gap: ~283k payout-stub payments carry no
-`:charge/client` attribute of their own, so it cannot verify them by attribute. `plan` resolves
-ownership through whatever refers to them and is the figure to trust.
+On the restored copy that is 5,159,787 orders — 27% of the table — via the
+`:sales-order/client+date` index. A year is chosen deliberately: `remove-voided-orders` only ever
+deletes orders Square reports as voided, which are recent, so that is where the destructive risk
+lives. Completeness across all of history is check (a)'s job, not this one.
+
+> Do **not** sample this with `(take n (rk/all-order-ids db))`. `all-order-ids` streams `:aevt`,
+> which is ascending entity id, so a `take` returns the *oldest* orders — on the restored copy the
+> first 400,000 are all from 2019–2021, before any of the contention this gate looks for. It would
+> report a confident zero having inspected none of the relevant data.
---
-## Step 5 — Recompute summaries, flags still off
+## Step 5 — Resume the Square importer
+
+Only once step 4's two checks read clean. The maintenance window ends here.
+
+The first cycle after resuming is the one to watch. Compare these against the same counts taken
+immediately before the deploy — growth should be ordinary daily volume:
+
+```clojure
+(count (d/datoms (d/db conn) :aevt :sales-refund/external-id))
+(count (d/datoms (d/db conn) :aevt :expected-deposit/external-id))
+(count (d/datoms (d/db conn) :aevt :cash-drawer-shift/external-id))
+(count (d/datoms (d/db conn) :aevt :charge/external-id))
+```
+
+A near-doubling of any of them means records are being created rather than matched — **stop and
+roll back the deploy.** Charges are included deliberately: they are the one that doubles a client's
+takings rather than merely duplicating a row.
+
+---
+
+## Step 6 — Recompute summaries, flags still off
```clojure
(require '[auto-ap.jobs.sales-summaries :as ss])
@@ -128,7 +183,7 @@ ownership through whatever refers to them and is the figure to trust.
```
This is the pass that banks the deduplication. **Capture the result before going further** — you
-will need it as the baseline for step 6, and it cannot be reconstructed afterwards:
+will need it as the baseline for step 7, and it cannot be reconstructed afterwards:
```clojure
(require '[auto-ap.tools.compare-sales-summaries :as cmp]) ; test/dev classpath
@@ -143,7 +198,7 @@ will need it as the baseline for step 6, and it cannot be reconstructed afterwar
---
-## Step 6 — Turn the flag on, a few restaurants at a time
+## Step 7 — Turn the flag on, a few restaurants at a time
Needs accounting sign-off first: `summary-service-charges` posts to **49000 Service Income**, chosen
so the work could be measured. It affects reporting, never whether a day balances.
@@ -156,7 +211,7 @@ so the work could be measured. It affects reporting, never whether a day balance
Start with two or three restaurants, confirm, then widen.
-**Verify** against the capture from step 5:
+**Verify** against the capture from step 6:
```clojure
(def after (cmp/summaries-in (d/db conn) start end))
@@ -168,29 +223,34 @@ The two numbers that matter — both were zero across all 18,900 client-days in
- `:balanced->unbalanced` must be **0**
- previously-balanced days whose lines changed must be **0**
-If either is non-zero, retract the flag for the affected clients and re-run step 5. The flag is the
+If either is non-zero, retract the flag for the affected clients and re-run step 6. The flag is the
rollback: removing it restores today's behaviour exactly.
---
-## Step 7 — Re-enable `remove-voided-orders`
+## Step 8 — Re-enable `remove-voided-orders`
-Safe once step 4's gate reads zero. Keep the detach-rather-than-delete guard from step 2.
+Safe once step 4's gate reads zero. Keep the detach-rather-than-delete guard from step 1.
---
-## Step 8 — Remove the legacy key lookup
+## Step 9 — Remove the legacy key lookup
-Only once `plan` reports `:to-migrate 0` and has stayed there through several import cycles. Drop
-the second branch of `square.core3/existing-id`. At that point two clients sharing a location
-becomes structurally incapable of producing a shared record, rather than prevented by a convention a
-future import could break.
+**Schedule this; do not leave it open-ended.** Both client records at a shared location stay active
+permanently, so the legacy fallback in `square.core3/existing-id` is the one code path that can ever
+reach across them. Deleting it is what turns the guarantee from conventional into structural.
-This is the last step and there is no hurry.
+Once `plan` reports `:to-migrate 0` and has stayed there through several import cycles, drop the
+legacy branch of `existing-id` — and with it `owned-by-other-client?`, which exists only to make
+that branch safe while it lives. After this, two clients on one location are structurally incapable
+of resolving onto each other's records, and no ordering discipline is required to keep it that way.
+
+Until it is done, the protection is the guard plus the maintenance window, both of which depend on
+people doing the right thing. That is the reason not to let this drift.
---
-## Step 9 — Deal with the refunds that have no sales behind them
+## Step 10 — Deal with the refunds that have no sales behind them
**The most important item in this document, and the only one that is not just execution.**
@@ -256,7 +316,7 @@ tender against $6,059.57 of order totals on one day), the ezCater fee question,
clusters on NGMV and NGEB.
Plus 15 days / $974.99 where a processing fee lands on a day with no trading — the same shape as
-step 9 but from the payout side, so it needs the payout modelled rather than a rule in the summary.
+step 10 but from the payout side, so it needs the payout modelled rather than a rule in the summary.
---
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 693fc55e..43aba8a0 100644
--- a/src/clj/auto_ap/jobs/rekey_square_external_ids.clj
+++ b/src/clj/auto_ap/jobs/rekey_square_external_ids.clj
@@ -195,7 +195,11 @@
[orders batch-size]
(let [cloned (atom 0)
rekeyed (atom 0)]
- (doseq [batch (partition-all batch-size orders)]
+ (doseq [[i batch] (map-indexed vector (partition-all batch-size orders))]
+ (when (zero? (mod i 200))
+ ;; the whole-database run walks 19M orders; without a trail an interrupted run leaves
+ ;; no way to tell how far it got short of querying the data by hand
+ (alog/info ::splitting :orders-done (* i batch-size) :rekeyed @rekeyed :cloned @cloned))
(let [db (dc/db conn)
batch-seen (atom {})
tx (doall
@@ -255,12 +259,19 @@
"Counts, per entity type, how many keys are already client-scoped, how many still carry the
legacy unscoped form, and how many have no owner to scope by.
- This is the completeness gate. The importer tolerates both key schemes on purpose, so that the
- change can be deployed before the migration finishes — but that tolerance is a transition, not
- a resting place. While `:legacy` is above zero the database is in a mixed state and a stray
- unscoped record can still be adopted by whichever client imports it first. Once every count
- reads zero the fallback lookup in `square.core3/existing-id` can be removed and the guarantee
- becomes structural rather than conventional."
+ The importer tolerates both key schemes on purpose, so that the change can be deployed before
+ the migration finishes — but that tolerance is a transition, not a resting place. While
+ `:legacy` is above zero the database is in a mixed state and a stray unscoped record can still
+ be adopted by whichever client imports it first. **`:legacy` reaching zero on every attribute is
+ the done-signal**, and it is what licenses removing the fallback lookup in
+ `square.core3/existing-id`.
+
+ `:no-owner` is NOT part of that signal and never reaches zero for charges. It counts entities
+ whose own `:charge/client`/`:charge/location` are absent — around an eighth of charges, the
+ payout stubs `migrate!` only ever gives an external id — so this report structurally cannot
+ verify them even when their keys are perfectly scoped. It classifies by attribute; `plan`
+ resolves ownership through whatever references the entity. Ask `plan` for the authoritative
+ answer: `:to-migrate 0` with `:unscopable 0` means there is nothing left to do."
[db]
(into {}
(for [{:keys [attr prefix client location]} scoped-attrs]
@@ -299,8 +310,13 @@
(when-let [c (seq (collisions (:new-keys p)))]
(throw (ex-info "two entities would take the same key" {:attr attr :collisions (count c)})))
(migrate! attr (:new-keys p) batch-size)))
- ;; charges no order refers to — payout stubs — are scoped from the deposit that holds them
+ ;; charges no order refers to — payout stubs — are scoped from the deposit that holds them.
+ ;; Collision-checked like the others: this is the largest attribute in the database, so it is
+ ;; the last one that should discover a clash as a mid-run exception.
(let [p (plan (dc/db conn) :charge/external-id charge-prefix)]
+ (when-let [c (seq (collisions (:new-keys p)))]
+ (throw (ex-info "two entities would take the same key"
+ {:attr :charge/external-id :collisions (count c)})))
(when (seq (:new-keys p)) (migrate! :charge/external-id (:new-keys p) batch-size)))
{:split split :completeness (unscoped-report (dc/db conn))}))
diff --git a/src/clj/auto_ap/square/core3.clj b/src/clj/auto_ap/square/core3.clj
index cfa7a7c0..97a2a329 100644
--- a/src/clj/auto_ap/square/core3.clj
+++ b/src/clj/auto_ap/square/core3.clj
@@ -27,11 +27,9 @@
"Authorization" (str "Bearer " (:client/square-auth-token client))
"Content-Type" "application/json"}))
-
(defn ->square-date [d]
(f/unparse (f/formatter "YYYY-MM-dd'T'HH:mm:ssZZ") d))
-
(def manifold-api-stream
(let [stream (s/stream 100)]
(->> stream
@@ -42,10 +40,10 @@
(de/loop [attempt 0]
(-> (de/chain (de/future-with (ex/execute-pool)
#_(log/info ::request-started
- :url (:url request)
- :attempt attempt
- :source "Square 3"
- :background-job "Square 3")
+ :url (:url request)
+ :attempt attempt
+ :source "Square 3"
+ :background-job "Square 3")
(try
(client/request (assoc request
:socket-timeout 10000
@@ -104,7 +102,6 @@
:exception error))
[]))))
-
(def item-cache (atom {}))
(defn fetch-catalog [client i v]
@@ -124,13 +121,11 @@
#(do (swap! item-cache assoc i %)
%))))
-
(defn fetch-catalog-cache [client i version]
(if (get @item-cache i)
(de/success-deferred (get @item-cache i))
(fetch-catalog client i version)))
-
(defn item->category-name-impl [client item version]
(capture-context->lc
(cond (:item_id (:item_variation_data item))
@@ -161,7 +156,6 @@
:item item)
"Uncategorized"))))
-
(defn item-id->category-name [client i version]
(capture-context->lc
(-> [client i]
@@ -226,7 +220,6 @@
(concat (:orders result) continued-results))))
(:orders result)))))))
-
(defn search
([client location start end]
(capture-context->lc
@@ -250,11 +243,9 @@
(concat (:orders result) continued-results))))
(:orders result))))))))
-
(defn amount->money [amt]
(* 0.01 (or (:amount amt) 0.0)))
-
;; to get totals:
(comment
(reduce
@@ -278,6 +269,30 @@
[prefix client location id]
(str prefix (:client/code client) "-" (:square-location/client-location location) "-" id))
+(def ^:private owner-attr
+ "Where each Square-imported entity records the client it belongs to."
+ {:charge/external-id :charge/client
+ :sales-refund/external-id :sales-refund/client
+ :expected-deposit/external-id :expected-deposit/client
+ :cash-drawer-shift/external-id :cash-drawer-shift/client})
+
+(defn- owned-by-other-client?
+ "Whether `e` already belongs to a client other than `client-eid`.
+
+ Reads the entity's own owner attribute, and for a charge falls back to the client of whichever
+ sales order refers to it — charges predating `:charge/client` still have orders, and those are
+ exactly the ones that can be taken by the wrong client."
+ [db attr e client-eid]
+ (let [ent (dc/entity db e)
+ owner (or (:db/id ((owner-attr attr) ent))
+ (when (= attr :charge/external-id)
+ (some->> (first (dc/datoms db :vaet e :sales-order/charges))
+ :e
+ (dc/entity db)
+ :sales-order/client
+ :db/id)))]
+ (and owner (not= owner client-eid))))
+
(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.
@@ -286,11 +301,24 @@
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."
+ whichever scheme it currently carries.
+
+ The legacy branch will not take a record that already belongs to a different client. Without
+ that check, two clients on one Square location double money during the window between deploying
+ and finishing the migration: client A's payout import resolves B's charge by its bare key and
+ renames it into A's scope, B's next order import then matches neither scheme and mints a second
+ charge, and because `:sales-order/charges` is cardinality-many nothing retracts the first — so
+ B's order carries two charges for one payment. Declining is also the right answer on its merits:
+ the write then lands on this client's own copy, which is what the scoped keys exist to create.
+
+ Once the migration has run there are no legacy keys left for this branch to find, and both it
+ and the guard can be deleted together."
[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)]))))
+ (when-let [legacy (dc/entid db [attr (str prefix id)])]
+ (when-not (owned-by-other-client? db attr legacy (:db/id client))
+ legacy)))))
(defn tender->charge [order client location t]
(remove-nils
@@ -304,7 +332,7 @@
:db/id (existing-id (dc/db conn) :charge/external-id "square/charge/" client location (:id t))
:external-id (when (:id t)
(scoped-key "square/charge/" client location (:id t)))
- :processor (cond
+ :processor (cond
(#{"OTHER" "THIRD_PARTY_CARD"} (:type t))
(condp = (some-> (:note t) str/lower-case)
"doordash" :ccp-processor/doordash
@@ -377,7 +405,7 @@
#:sales-order
{:date (if (= "Invoices" (:name (:source order)))
(when (:closed_at order)
- (coerce/to-date (time/to-time-zone (coerce/to-date-time (:closed_at order)) (time/time-zone-for-id "America/Los_Angeles"))))
+ (coerce/to-date (time/to-time-zone (coerce/to-date-time (:closed_at order)) (time/time-zone-for-id "America/Los_Angeles"))))
(coerce/to-date (time/to-time-zone (coerce/to-date-time (:created_at order)) (time/time-zone-for-id "America/Los_Angeles"))))
:client (:db/id client)
:location (:square-location/client-location location)
@@ -439,7 +467,6 @@
:client client
:location location)))))))
-
(defn get-payment [client p]
(de/chain (manifold-api-call
{:url (str "https://connect.squareup.com/v2/payments/" p)
@@ -448,7 +475,6 @@
:body
:payment))
-
(defn continue-payout-entry-list [c l poi cursor]
(capture-context->lc lc
(de/chain
@@ -593,6 +619,12 @@
(let [payment-id (:payment_id (:type_charge_details p))]
(remove-nils
{:charge/external-id (scoped-key "square/charge/" client location payment-id)
+ ;; the owner attributes must travel with the key: `raw-square-id` and
+ ;; `scope-of` both recover a charge's scope from them, and a key
+ ;; scoped to one client while the owner says another is what makes
+ ;; the migration write square/charge/B-LB-A-LA-
+ :charge/client (:db/id client)
+ :charge/location (:square-location/client-location location)
:db/id (existing-id (dc/db conn) :charge/external-id "square/charge/" client location payment-id)}))))))})
(filter :expected-deposit/date)
(into []))
@@ -651,7 +683,6 @@
:count (count x))
@(dc/transact-async conn x))))))))
-
(defn upsert-payouts
([client]
(apply de/zip
@@ -700,7 +731,6 @@
(log/info ::done-loading-refunds)))))))
-
(defn get-cash-shift [client id]
(de/chain (manifold-api-call {:url (str (url/url "https://connect.squareup.com/v2/cash-drawers/shifts" id))
:method :get
@@ -864,8 +894,6 @@
d1
d2))
-
-
(defn remove-voided-orders
([client]
(apply de/zip
@@ -892,7 +920,7 @@
(:sales-order/external-id o))))))
(s/map (fn [[o]]
[[:db/retractEntity [:sales-order/external-id (:sales-order/external-id o)]]]))
-
+
(s/reduce into [])))
(fn [results]
@@ -901,31 +929,26 @@
(log/info ::removing-orders
:count (count x))
@(dc/transact-async conn x)))))
- (de/catch (fn [e]
- (log/warn ::couldnt-remove :error e)
- nil) ))))))
+ (de/catch (fn [e]
+ (log/warn ::couldnt-remove :error e)
+ nil)))))))
-#_(comment
- (require 'auto-ap.time-reader)
+#_(comment
+ (require 'auto-ap.time-reader)
- @(let [[c [l]] (get-square-client-and-location "DBFS") ]
- (log/peek :x [ c l])
- (search c l #clj-time/date-time "2026-03-28" #clj-time/date-time "2026-03-29")
+ @(let [[c [l]] (get-square-client-and-location "DBFS")]
+ (log/peek :x [c l])
+ (search c l #clj-time/date-time "2026-03-28" #clj-time/date-time "2026-03-29"))
- )
+ @(let [[c [l]] (get-square-client-and-location "NGAK")]
+ (log/peek :x [c l])
- @(let [[c [l]] (get-square-client-and-location "NGAK") ]
- (log/peek :x [ c l])
-
- (remove-voided-orders c l #clj-time/date-time "2024-04-11" #clj-time/date-time "2024-04-15"))
- (doseq [c (get-square-clients)]
- (try
- @(remove-voided-orders c)
- (catch Exception e
- nil)))
-
-
- )
+ (remove-voided-orders c l #clj-time/date-time "2024-04-11" #clj-time/date-time "2024-04-15"))
+ (doseq [c (get-square-clients)]
+ (try
+ @(remove-voided-orders c)
+ (catch Exception e
+ nil))))
(defn upsert-all [& clients]
(capture-context->lc
@@ -994,8 +1017,6 @@
[:clients clients]
@(apply upsert-all clients)))
-
-
(comment
(defn refunds-raw-cont
([client l cursor so-far]
@@ -1025,9 +1046,8 @@
(->>
@(let [[c [l]] (get-square-client-and-location "NGGG")]
-
(search c l (time/now) (time/plus (time/now) (time/days -1))))
-
+
(filter (fn [r]
(str/starts-with? (:created_at r) "2024-03-14"))))
@@ -1035,7 +1055,6 @@
(->>
@(let [[c [l]] (get-square-client-and-location "NGGG")]
-
(refunds-raw-cont c l nil []))
(filter (fn [r]
(str/starts-with? (:created_at r) "2024-03-14")))))
@@ -1069,13 +1088,8 @@
[]))]
[(:client/code c) (atime/unparse-local (clj-time.coerce/to-date-time (:sales-order/date bad-row)) atime/normal-date) (:sales-order/total bad-row) (:sales-order/tax bad-row) (:sales-order/tip bad-row) (:db/id bad-row)])
:separator \tab)
-
-
-
-
- ;; =>
-
+;; =>
(require 'auto-ap.time-reader)
@@ -1084,27 +1098,16 @@
(clojure.pprint/pprint (let [[c [l]] (get-square-client-and-location "NGVT")]
l
-
(def z @(search c l #clj-time/date-time "2025-02-23T00:00:00-08:00"
#clj-time/date-time "2025-02-28T00:00:00-08:00"))
- (take 10 (map #(first (deref (order->sales-order c l %))) z)))
+ (take 10 (map #(first (deref (order->sales-order c l %))) z))))
-
- )
-
-
-
-
- (->> z
+ (->> z
(filter (fn [o]
(seq (filter (comp #{"OTHER"} :type) (:tenders o)))))
(filter #(not (:name (:source %))))
- (count)
-
- )
-
-
-
+ (count))
+
(doseq [[code] (seq (dc/q '[:find ?code
:in $
:where [?o :sales-order/date ?d]
@@ -1113,32 +1116,22 @@
[?o :sales-order/client ?c]
[?c :client/code ?code]]
(dc/db conn)))
- :let [[c [l]] (get-square-client-and-location code)
- ]
+ :let [[c [l]] (get-square-client-and-location code)]
order @(search c l #clj-time/date-time "2026-01-01T00:00:00-08:00" (time/now))
- :when (= "Invoices" (:name (:source order) ))
+ :when (= "Invoices" (:name (:source order)))
:let [[sales-order] @(order->sales-order c l order)]]
-
+
(when (should-import-order? order)
(println "DATE IS" (:sales-order/date sales-order))
(when (some-> (:sales-order/date sales-order) coerce/to-date-time (time/after? #clj-time/date-time "2026-2-16T00:00:00-08:00"))
(println "WOULD UPDATE" sales-order)
- @(dc/transact auto-ap.datomic/conn [sales-order])
- )
- #_@(dc/transact )
- (println "DONE"))
-
-
- )
+ @(dc/transact auto-ap.datomic/conn [sales-order]))
+ #_@(dc/transact)
+ (println "DONE")))
#_(filter (comp #{"OTHER"} :type) (mapcat :tenders z))
-
@(let [[c [l]] (get-square-client-and-location "NGRY")]
#_(search c l (clj-time.coerce/from-date #inst "2025-02-28") (clj-time.coerce/from-date #inst "2025-03-01"))
- (order->sales-order c l (:order (get-order c l "KdvwntmfMNTKBu8NOocbxatOs18YY" )))
-
- )
-
- )
+ (order->sales-order c l (:order (get-order c l "KdvwntmfMNTKBu8NOocbxatOs18YY")))))
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 a5825462..cc0f646e 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
@@ -147,3 +147,30 @@
(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"))))
+
+(deftest two-orders-of-the-same-client-keep-sharing-across-batches
+ (testing "batch size does not change the same-client rule, which the sibling test cannot show
+ because both its orders land in one batch.
+
+ Once the first order re-keys the charge it also writes :charge/client/:charge/location,
+ so the second order's raw-square-id takes its owner branch, new-key reconstructs the key
+ the charge already has, and the (not= old new-key) guard drops the row before :action is
+ read. A clone here would double that client's takings for the day."
+ (let [{:strs [test-client-id]} (setup-test-data [])
+ tx @(dc/transact conn [{:db/id "charge"
+ :charge/external-id "square/charge/same2"
+ :charge/total 75.0}
+ {:db/id "o1" :sales-order/external-id "square/order/y-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/y-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] 1))
+ "batch size 1 puts the two orders in separate batches, and still no copy is made")
+ (is (= 1 (charge-count)) "one payment, not two")
+ (is (= (str "square/charge/" code "-CD-same2") (:key (first (charges-of o1)))))
+ (is (= (:eid (first (charges-of o1))) (:eid (first (charges-of o2))))
+ "both orders still point at the one payment"))))
diff --git a/test/clj/auto_ap/square/core3_test.clj b/test/clj/auto_ap/square/core3_test.clj
index b99eeeef..6d772fd7 100644
--- a/test/clj/auto_ap/square/core3_test.clj
+++ b/test/clj/auto_ap/square/core3_test.clj
@@ -76,6 +76,89 @@
:sales-refund/total 10.0}])
(is (= 2 (refund-count)) "two stable entities, one per client, rather than one that flips"))))
+(deftest legacy-key-of-another-client-is-not-claimed
+ (testing "the legacy fallback must not hand one client a record that already belongs to another.
+
+ Without this, two clients on one Square location double money in the window between
+ deploying and finishing the migration."
+ (setup-test-data [])
+ (let [tx @(dc/transact conn [{:db/id "mine" :client/code (str "MINE" (rand-int 100000))}
+ {:db/id "theirs" :client/code (str "THEIRS" (rand-int 100000))}])
+ mine {:db/id (get-in tx [:tempids "mine"]) :client/code "MINE"}
+ theirs-id (get-in tx [:tempids "theirs"])]
+ @(dc/transact conn [{:db/id "r"
+ :sales-refund/external-id "square/refund/abc"
+ :sales-refund/client theirs-id
+ :sales-refund/total 10.0}])
+ (is (nil? (sut/existing-id (dc/db conn) :sales-refund/external-id "square/refund/"
+ mine location "abc"))
+ "a legacy-keyed refund owned by another client is left alone")
+ (is (some? (sut/existing-id (dc/db conn) :sales-refund/external-id "square/refund/"
+ {:db/id theirs-id :client/code "THEIRS"} location "abc"))
+ "its own client still resolves it, so re-keying in place still works"))))
+
+(deftest a-charge-is-owned-by-the-client-of-the-order-that-refers-to-it
+ (testing "charges predating :charge/client still have orders, and those are exactly the ones
+ that could be taken by the wrong client"
+ (let [{:strs [test-client-id]} (setup-test-data [])
+ other (get-in @(dc/transact conn [{:db/id "o" :client/code (str "OTHER" (rand-int 100000))}])
+ [:tempids "o"])]
+ @(dc/transact conn [{:db/id "c" :charge/external-id "square/charge/p1" :charge/total 50.0}
+ {:db/id "ord" :sales-order/external-id "square/order/x-1"
+ :sales-order/client test-client-id :sales-order/location "CD"
+ :sales-order/date #inst "2026-06-03T07:00:00.000-00:00"
+ :sales-order/charges ["c"]}])
+ (is (nil? (sut/existing-id (dc/db conn) :charge/external-id "square/charge/"
+ {:db/id other :client/code "OTHER"} location "p1"))
+ "ownership is read from the referencing order when :charge/client is absent"))))
+
+(deftest deploy-window-does-not-double-a-second-clients-tender
+ (testing "the P0 this guard exists for, end to end.
+
+ Client A's payout import reaches for a payment whose charge belongs to client B's
+ order. If A were allowed to re-key it, B's next order import would match neither
+ scheme, mint a second charge, and — since :sales-order/charges is cardinality-many —
+ leave B's order holding two charges for one payment."
+ (let [{:strs [test-client-id]} (setup-test-data [])
+ b-code (:client/code (dc/entity (dc/db conn) test-client-id))
+ b {:db/id test-client-id :client/code b-code}
+ b-loc {:square-location/client-location "LB"}
+ a-id (get-in @(dc/transact conn [{:db/id "a" :client/code (str "AAA" (rand-int 100000))}])
+ [:tempids "a"])
+ a {:db/id a-id :client/code (:client/code (dc/entity (dc/db conn) a-id))}
+ a-loc {:square-location/client-location "LA"}
+ tx @(dc/transact conn [{:db/id "x" :charge/external-id "square/charge/P"
+ :charge/total 100.0 :charge/type-name "CARD"
+ :charge/client test-client-id :charge/location "LB"}
+ {:db/id "ob" :sales-order/external-id "square/order/b-1"
+ :sales-order/client test-client-id :sales-order/location "LB"
+ :sales-order/date #inst "2026-06-03T07:00:00.000-00:00"
+ :sales-order/charges ["x"]}])
+ order-b (get-in tx [:tempids "ob"])
+ charges-of (fn [o] (map :v (dc/datoms (dc/db conn) :eavt o :sales-order/charges)))]
+ ;; client A's payout import touches the same Square payment
+ @(dc/transact conn [(into {} (remove (comp nil? val))
+ {:charge/external-id (sut/scoped-key "square/charge/" a a-loc "P")
+ :charge/client a-id
+ :charge/location "LA"
+ :db/id (sut/existing-id (dc/db conn) :charge/external-id
+ "square/charge/" a a-loc "P")})])
+ ;; client B's order re-imports
+ @(dc/transact conn [{:db/id order-b
+ :sales-order/charges
+ [(sut/tender->charge {:id "b-1" :created_at "2026-06-03T12:00:00Z"}
+ b b-loc {:id "P" :type "CARD"
+ :amount_money {:amount 10000
+ :currency "USD"}})]}])
+ (is (= 1 (count (charges-of order-b)))
+ "B's order still holds exactly one charge for the one payment")
+ (is (= 100.0 (reduce + 0.0 (map #(:charge/total (dc/entity (dc/db conn) %))
+ (charges-of order-b))))
+ "so the day's tender is not doubled")
+ (is (= (str "square/charge/" b-code "-LB-P")
+ (:charge/external-id (dc/entity (dc/db conn) (first (charges-of order-b)))))
+ "and B's own charge was re-keyed in place rather than abandoned"))))
+
(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"