From 9e4cb851ff9d1a6473c3bbec1d35a34d57dc629e Mon Sep 17 00:00:00 2001 From: Bryce Date: Fri, 14 Aug 2026 23:29:14 -0700 Subject: [PATCH] fix(sales-summaries): count reversed tips, credit Square service charges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two calculation defects that leave a day out of balance, plus the first tests to cover sales summaries. Tips: get-tip joins through :sales-order/charges, so it only sees tips that settled on a tender. A return-only order has no tender — it carries the reversal on :sales-order/tip, which nothing read — so the day credited a tip that had been handed back. Now additive: tendered tips plus the tips on orders that have no tender at all. Deliberately not a swap, because where an order does have a tender the tender is the correct source; real orders exist whose tender carries a tip their :sales-order/tip does not (auto-gratuity booked as a service charge, wallet tips missing from the net amounts), and reading the order would drop them. Service charges: nothing reads :sales-order/service-charge. The charge is collected inside the card tender but no line credits it, so every order carrying one leaves the day short by exactly that amount. Both signs count — a returned catering fee arrives as a negative service charge and is subtracted back out of :sales-order/returns, so dropping negatives would lose the reversal. The vendor gate is load-bearing: ezCater service charges are commission deducted from the restaurant rather than collected from the diner, and crediting those makes the day worse. It matches on :sales-order/vendor where that is set and falls back to the external id prefix where it is not — whole eras of Square orders carry no vendor attribute at all, and a gate on vendor alone would have silently credited nothing. Service charges stay behind the per-client "summary-service-charges" flag, and the account they map to (49000 Service Income) needs accounting sign-off before that flag is enabled anywhere. Co-Authored-By: Claude Opus 5 --- src/clj/auto_ap/jobs/sales_summaries.clj | 131 ++++++++++++++++-- .../clj/auto_ap/jobs/sales_summaries_test.clj | 122 ++++++++++++++++ 2 files changed, 242 insertions(+), 11 deletions(-) create mode 100644 test/clj/auto_ap/jobs/sales_summaries_test.clj diff --git a/src/clj/auto_ap/jobs/sales_summaries.clj b/src/clj/auto_ap/jobs/sales_summaries.clj index 52391a1b..ef68a69d 100644 --- a/src/clj/auto_ap/jobs/sales_summaries.clj +++ b/src/clj/auto_ap/jobs/sales_summaries.clj @@ -147,8 +147,18 @@ date)) 0.0))) +(def service-charges-account + "Where a credited Square service charge lands. 49000 is the existing \"Service Income\" + revenue account, which is the closest fit for auto-gratuity and catering fees. + + NEEDS ACCOUNTING SIGN-OFF before `service-charges-flag` is enabled for any client: the wrong + account misstates revenue, and a category with no account at all keeps a day from ever + reaching accepted, since `accepted?` requires every line to be mapped." + 49000) + (def name->number {"gyros and pitas" 40111 + "service charges" service-charges-account "returns" 41300 "card payments" 75460 "cash payments" 75452 @@ -292,21 +302,48 @@ [[c] date date])) 0.0)}) -(defn- get-tip [c date] +(defn- tendered-tip + "Tips read off the tenders, which is where a tip actually settles." + [c date] + (or (ffirst (dc/q '[:find (sum ?tip) + :with ?c + :in $ [?clients ?start-date ?end-date] + :where [(iol-ion.query/scan-sales-orders $ ?clients ?start-date ?end-date) [[?e _ ?sort-default] ...]] + [?e :sales-order/charges ?c] + [?c :charge/tip ?tip]] + (dc/db conn) + [[c] date date])) + 0.0)) + +(defn- untendered-tip + "Tips on orders that carry no tender at all. A return-only order reverses its tip on + `:sales-order/tip` but has no charge to join through, so the reversal is invisible to + `tendered-tip` and the day ends up crediting a tip that was handed back." + [c date] + (or (ffirst (dc/q '[:find (sum ?tip) + :with ?e + :in $ [?clients ?start-date ?end-date] + :where [(iol-ion.query/scan-sales-orders $ ?clients ?start-date ?end-date) [[?e _ ?sort-default] ...]] + [?e :sales-order/tip ?tip] + (not [?e :sales-order/charges])] + (dc/db conn) + [[c] date date])) + 0.0)) + +(defn- get-tip + "Tendered tips plus the tips on untendered orders. Additive rather than substitutive on + purpose: where an order does have a tender, the tender is the correct source, and real + orders exist whose tender carries a tip their `:sales-order/tip` does not — auto-gratuity + booked as a service charge, and wallet tips absent from the net amounts. Reading the order + instead of the tender would drop those." + [c date] {:ledger-mapped/ledger-side :ledger-side/credit :sales-summary-item/sort-order 2 :db/id (str (java.util.UUID/randomUUID)) :sales-summary-item/category "Tip" - :ledger-mapped/amount (or (ffirst (dc/q '[:find (sum ?tip) - :with ?c - :in $ [?clients ?start-date ?end-date] - :where [(iol-ion.query/scan-sales-orders $ ?clients ?start-date ?end-date) [[?e _ ?sort-default] ...]] - [?e :sales-order/charges ?c] - [?c :charge/tip ?tip]] - (dc/db conn) - [[c] date date])) - 0.0)}) + :ledger-mapped/amount (+ (tendered-tip c date) + (untendered-tip c date))}) (defn- get-sales [c date] (let [sales (->> (dc/q '[:find ?category (sum ?total) (sum ?tax) (sum ?discount) @@ -349,6 +386,74 @@ :ledger-mapped/amount amount :ledger-mapped/ledger-side :ledger-side/debit})) +(def service-charges-flag + "Per-client rollout lever for crediting Square service charges, in the same style as + `new-square` and `import-custom-amount`. Absent, the summary behaves exactly as it does + today." + "summary-service-charges") + +(defn- service-charges-enabled? [c] + (contains? (set (:client/feature-flags (dc/pull (dc/db conn) '[:client/feature-flags] c))) + service-charges-flag)) + +(defn service-charge-total + "Square service charges for the day, both signs. + + A service charge is collected inside the card tender but nothing credits it, so every order + carrying one leaves the day short by exactly that amount. Both signs matter: a returned + catering fee arrives as a negative service charge and is subtracted back out of + `:sales-order/returns`, so dropping negatives would lose the reversal. + + The vendor gate is load-bearing — ezCater service charges are commission deducted from the + restaurant rather than collected from the diner, and crediting those would make things worse. + It matches on `:sales-order/vendor` where that is set and falls back to the external id + prefix where it is not, because whole eras of Square orders carry no vendor attribute at all + and a gate on vendor alone silently credits nothing. + + Kept separate from the rollout flag so the arithmetic can be measured on its own." + [c date] + (ffirst (dc/q '[:find (sum ?service-charge) + :with ?e + :in $ [?clients ?start-date ?end-date] + :where [(iol-ion.query/scan-sales-orders $ ?clients ?start-date ?end-date) [[?e _ ?sort-default] ...]] + [?e :sales-order/service-charge ?service-charge] + (or-join [?e] + [?e :sales-order/vendor :vendor/ccp-square] + (and (not [?e :sales-order/vendor]) + [?e :sales-order/external-id ?external-id] + [(clojure.string/starts-with? ?external-id "square/order/")]))] + (dc/db conn) + [[c] date date]))) + +(defn- get-service-charges + "The day's service charges as a summary item, for clients opted in to the rollout." + [c date] + (when (service-charges-enabled? c) + (when-let [amount (service-charge-total c date)] + (when-not (zero? amount) + {:db/id (str (java.util.UUID/randomUUID)) + :sales-summary-item/category "Service Charges" + :sales-summary-item/sort-order 2 + :ledger-mapped/amount amount + :ledger-mapped/ledger-side :ledger-side/credit})))) + +(def ^:private suspect-categories + "The terms a balancing investigation keeps returning to. Logged beside the imbalance so a + day's shape can be read out of the logs without re-running the job." + ["Tip" "Service Charges" "Returns" "Card Refunds" "Cash Refunds" "Food App Refunds"]) + +(defn- suspect-totals + "Amounts for `suspect-categories` present on this day, omitting the ones that are zero." + [items] + (into {} + (for [category suspect-categories + :let [amount (->> items + (filter #(= category (:sales-summary-item/category %))) + (map #(:ledger-mapped/amount % 0.0)) + (reduce + 0.0))] + :when (not (zero? amount))] + [category amount]))) + (defn sales-summaries-v2 [] (doseq [[c client-code] (dc/q '[:find ?c ?client-code :in $ @@ -370,6 +475,7 @@ (cons (get-fees c date)) (cons (get-tax c date)) (cons (get-tip c date)) + (cons (get-service-charges c date)) (cons (get-returns c date)) (filter identity) (map (fn [z] @@ -385,7 +491,10 @@ (if (seq (:sales-summary/items result)) (do (alog/info ::upserting-summaries - :category-count (count (:sales-summary/items result))) + :category-count (count (:sales-summary/items result)) + :imbalance (d-ss/imbalance all-items) + :balanced? (d-ss/balanced? all-items) + :suspect-totals (suspect-totals all-items)) @(dc/transact conn [[:upsert-sales-summary result]])) @(dc/transact conn [{:db/id id :sales-summary/dirty false}])))))) diff --git a/test/clj/auto_ap/jobs/sales_summaries_test.clj b/test/clj/auto_ap/jobs/sales_summaries_test.clj new file mode 100644 index 00000000..74b8b6dc --- /dev/null +++ b/test/clj/auto_ap/jobs/sales_summaries_test.clj @@ -0,0 +1,122 @@ +(ns auto-ap.jobs.sales-summaries-test + (:require + [auto-ap.datomic :refer [conn]] + [auto-ap.integration.util :refer [setup-test-data wrap-setup]] + [auto-ap.jobs.sales-summaries :as sut] + [clojure.test :refer [deftest is testing use-fixtures]] + [datomic.api :as dc])) + +(use-fixtures :each wrap-setup) + +(def sales-date #inst "2026-08-01T07:00:00.000-00:00") + +(defn- order + "A sales order on `sales-date`, carrying whatever the case under test needs. The external id + is Square-shaped by default because `get-service-charges` falls back to it when an order has + no `:sales-order/vendor`." + [client id attrs] + (merge {:db/id (str "order-" id) + :sales-order/external-id (str "square/order/TEST-" id) + :sales-order/client client + :sales-order/date sales-date + :sales-order/total 100.0} + attrs)) + +(defn- charge [id attrs] + (merge {:db/id (str "charge-" id) + :charge/external-id (str "square/charge/" id) + :charge/type-name "CARD" + :charge/total 100.0} + attrs)) + +(defn- tip-for [client] + (:ledger-mapped/amount (#'sut/get-tip client sales-date))) + +(defn- service-charges-for [client] + (#'sut/get-service-charges client sales-date)) + +(defn- enable-service-charges! [client] + @(dc/transact conn [{:db/id client + :client/feature-flags [sut/service-charges-flag]}])) + +(deftest tip-counts-a-reversal-on-an-untendered-order + (testing "a return-only order has no tender to join through, so its negative tip must come + from the order or the day credits a tip that was handed back" + (let [{:strs [test-client-id]} (setup-test-data [])] + @(dc/transact conn [(order test-client-id "return-only" {:sales-order/tip -12.0})]) + (is (= -12.0 (tip-for test-client-id)))))) + +(deftest tip-on-a-tendered-order-still-comes-from-the-tender + (testing "the tender carries a tip the order does not — auto-gratuity booked as a service + charge. Reading the order instead of the tender would drop it." + (let [{:strs [test-client-id]} (setup-test-data [])] + @(dc/transact conn [(order test-client-id "tendered" + {:sales-order/tip 0.0 + :sales-order/charges [(charge "tendered" {:charge/tip 50.0})]})]) + (is (= 50.0 (tip-for test-client-id)))))) + +(deftest tip-on-an-ordinary-order-is-counted-once + (testing "an order that agrees with its tender is not double counted by the additive form" + (let [{:strs [test-client-id]} (setup-test-data [])] + @(dc/transact conn [(order test-client-id "ordinary" + {:sales-order/tip 5.0 + :sales-order/charges [(charge "ordinary" {:charge/tip 5.0})]})]) + (is (= 5.0 (tip-for test-client-id)))))) + +(deftest service-charges-need-the-feature-flag + (testing "without the flag the summary behaves exactly as it does today" + (let [{:strs [test-client-id]} (setup-test-data [])] + @(dc/transact conn [(order test-client-id "square-sc" + {:sales-order/vendor :vendor/ccp-square + :sales-order/service-charge 50.0})]) + (is (nil? (service-charges-for test-client-id)))))) + +(deftest service-charges-credit-square-orders + (testing "a service charge rides along in the tender, so it needs a credit to match" + (let [{:strs [test-client-id]} (setup-test-data [])] + (enable-service-charges! test-client-id) + @(dc/transact conn [(order test-client-id "square-sc" + {:sales-order/vendor :vendor/ccp-square + :sales-order/service-charge 50.0})]) + (let [item (service-charges-for test-client-id)] + (is (= 50.0 (:ledger-mapped/amount item))) + (is (= :ledger-side/credit (:ledger-mapped/ledger-side item))) + (is (= "Service Charges" (:sales-summary-item/category item))))))) + +(deftest service-charges-count-both-signs + (testing "a returned catering fee arrives as a negative service charge and is subtracted back + out of returns, so dropping negatives loses the reversal" + (let [{:strs [test-client-id]} (setup-test-data [])] + (enable-service-charges! test-client-id) + @(dc/transact conn [(order test-client-id "refunded-fee" + {:sales-order/vendor :vendor/ccp-square + :sales-order/service-charge -140.0})]) + (is (= -140.0 (:ledger-mapped/amount (service-charges-for test-client-id))))))) + +(deftest service-charges-exclude-non-square-vendors + (testing "ezCater service charges are commission deducted from the restaurant rather than + collected from the diner, so crediting them would make the day worse" + (let [{:strs [test-client-id]} (setup-test-data [])] + (enable-service-charges! test-client-id) + @(dc/transact conn [(order test-client-id "ezcater-sc" + {:sales-order/external-id "ezcater/order/TEST-ezcater-sc" + :sales-order/vendor :vendor/ccp-ezcater + :sales-order/service-charge -75.0})]) + (is (nil? (service-charges-for test-client-id)))))) + +(deftest service-charges-recognise-square-orders-that-carry-no-vendor + (testing "whole eras of Square orders have no :sales-order/vendor at all; a gate on vendor + alone would silently credit nothing" + (let [{:strs [test-client-id]} (setup-test-data [])] + (enable-service-charges! test-client-id) + @(dc/transact conn [(order test-client-id "vendorless" {:sales-order/service-charge 12.5})]) + (is (= 12.5 (:ledger-mapped/amount (service-charges-for test-client-id))))))) + +(deftest service-charges-ignore-vendorless-orders-from-other-sources + (testing "the external id fallback is Square-specific, not a catch-all for missing vendors" + (let [{:strs [test-client-id]} (setup-test-data [])] + (enable-service-charges! test-client-id) + @(dc/transact conn [(order test-client-id "ezcater-vendorless" + {:sales-order/external-id "ezcater/order/TEST-ezcater-vendorless" + :sales-order/service-charge -75.0})]) + (is (nil? (service-charges-for test-client-id))))))