From a17e16e31be0b842d28c3b1232bebf25cf091c66 Mon Sep 17 00:00:00 2001 From: Bryce Date: Tue, 4 Aug 2026 22:44:52 -0700 Subject: [PATCH] feat(ledger): remove a whole ledger entry from the external import grid The external ledger import review grid is one row per line item, so a journal entry spans two or more rows. When one entry fails validation the whole paste is rejected, and the only way forward was to re-paste without it. Each row now carries its entry id (client-source-externalId, the same key table->entries groups on) and the last column gets a trash button that drops every row sharing that id. The rows are the form inputs, so removing them from the DOM removes them from the next import post -- which also works for rows that fail schema validation. Index gaps left behind are compacted by coerce-vector on the way back in. Also drops two leftover pprint calls that dumped form-errors to stdout on every render. Co-Authored-By: Claude Opus 5 --- src/clj/auto_ap/ssr/ledger.clj | 178 +++++++++++++++------------ test/clj/auto_ap/ssr/ledger_test.clj | 91 ++++++++++++++ 2 files changed, 190 insertions(+), 79 deletions(-) diff --git a/src/clj/auto_ap/ssr/ledger.clj b/src/clj/auto_ap/ssr/ledger.clj index 09bf1152..f1250eb4 100644 --- a/src/clj/auto_ap/ssr/ledger.clj +++ b/src/clj/auto_ap/ssr/ledger.clj @@ -148,14 +148,15 @@ (= ::route/external-page matched-current-page-route) (assoc-in [:route-params :external?] true))] (handler request)))) +(defn line->id [{:keys [source external-id client-code]}] + (str client-code "-" source "-" external-id)) + (defn external-import-table-form* [request] [:div#table-form - (clojure.pprint/pprint (:form-errors request)) (fc/start-form (:form-params request) (:form-errors request) (fc/with-field :table - (clojure.pprint/pprint (fc/field-errors)) (when (seq (fc/field-value)) [:div {:x-data (hx/json {"showTable" false})} @@ -198,81 +199,90 @@ :rows (fc/cursor-map (fn [r] - (com/data-grid-row {} (com/data-grid-cell {} - (fc/with-field :external-id - (com/validated-field - {:errors (fc/field-errors)} - (com/text-input {:value (fc/field-value) - :name (fc/field-name)})))) - (com/data-grid-cell {} - (fc/with-field :client-code - (com/validated-field - {:errors (fc/field-errors)} - (com/text-input {:value (fc/field-value) - :name (fc/field-name) - :class "w-24"})))) - (com/data-grid-cell {} - (fc/with-field :source - (com/validated-field - {:errors (fc/field-errors)} - (com/text-input - {:value (fc/field-value) - :name (fc/field-name)})))) - (com/data-grid-cell {} (fc/with-field :vendor-name - (com/validated-field - {:errors (fc/field-errors)} - (com/text-input {:value (fc/field-value) - :name (fc/field-name)})))) - (com/data-grid-cell {} (fc/with-field :date - (com/validated-field - {:errors (fc/field-errors)} - (com/text-input {:value (some-> (fc/field-value) (atime/unparse-local - atime/normal-date)) - :name (fc/field-name) - :class "w-24"})))) - (com/data-grid-cell {} - (fc/with-field :account-code - (com/validated-field {:errors (fc/field-errors)} - (com/text-input {:value (fc/field-value) - :name (fc/field-name) - :class "w-16"})))) - (com/data-grid-cell {} (fc/with-field :location - (com/validated-field - {:errors (fc/field-errors)} - (com/text-input {:value (fc/field-value) - :name (fc/field-name) - :size 2})))) - (com/data-grid-cell {} (fc/with-field :debit - (com/validated-field {:errors (fc/field-errors)} - (com/money-input {:value (fc/field-value) - :name (fc/field-name) - :class "w-24"})))) - (com/data-grid-cell {} (fc/with-field :credit - (com/validated-field {:errors (fc/field-errors)} - (com/money-input {:value (fc/field-value) - :name (fc/field-name) - :class "w-24"})))) - (com/data-grid-cell {:class "align-top"} - [:div.p-2 - (let [errors (seq (fc/field-errors))] - (cond errors - [:div - {"x-tooltip" "{content: ()=>$refs.tt.innerHTML , allowHTML: true}"} - [:div.w-8.h-8.rounded-full.p-2.flex.items-start {:class - (if (seq (filter - (fn [[_ status]] + (let [entry-id (line->id (fc/field-value r))] + (com/data-grid-row {:data-entry-id entry-id} + (com/data-grid-cell {} + (fc/with-field :external-id + (com/validated-field + {:errors (fc/field-errors)} + (com/text-input {:value (fc/field-value) + :name (fc/field-name)})))) + (com/data-grid-cell {} + (fc/with-field :client-code + (com/validated-field + {:errors (fc/field-errors)} + (com/text-input {:value (fc/field-value) + :name (fc/field-name) + :class "w-24"})))) + (com/data-grid-cell {} + (fc/with-field :source + (com/validated-field + {:errors (fc/field-errors)} + (com/text-input + {:value (fc/field-value) + :name (fc/field-name)})))) + (com/data-grid-cell {} (fc/with-field :vendor-name + (com/validated-field + {:errors (fc/field-errors)} + (com/text-input {:value (fc/field-value) + :name (fc/field-name)})))) + (com/data-grid-cell {} (fc/with-field :date + (com/validated-field + {:errors (fc/field-errors)} + (com/text-input {:value (some-> (fc/field-value) (atime/unparse-local + atime/normal-date)) + :name (fc/field-name) + :class "w-24"})))) + (com/data-grid-cell {} + (fc/with-field :account-code + (com/validated-field {:errors (fc/field-errors)} + (com/text-input {:value (fc/field-value) + :name (fc/field-name) + :class "w-16"})))) + (com/data-grid-cell {} (fc/with-field :location + (com/validated-field + {:errors (fc/field-errors)} + (com/text-input {:value (fc/field-value) + :name (fc/field-name) + :size 2})))) + (com/data-grid-cell {} (fc/with-field :debit + (com/validated-field {:errors (fc/field-errors)} + (com/money-input {:value (fc/field-value) + :name (fc/field-name) + :class "w-24"})))) + (com/data-grid-cell {} (fc/with-field :credit + (com/validated-field {:errors (fc/field-errors)} + (com/money-input {:value (fc/field-value) + :name (fc/field-name) + :class "w-24"})))) + (com/data-grid-cell {:class "align-top"} + [:div.p-2.flex.items-start.gap-2 + (let [errors (seq (fc/field-errors))] + (cond errors + [:div + {"x-tooltip" "{content: ()=>$refs.tt.innerHTML , allowHTML: true}"} + [:div.w-8.h-8.rounded-full.p-2.flex.items-start {:class + (if (seq (filter + (fn [[_ status]] - (= :error status)) - errors)) - "bg-red-50 text-red-300" - "bg-yellow-100 text-yellow-600")} - svg/alert] - [:template {:x-ref "tt"} - [:ul - (for [[m] errors] - [:li m])]]] - :else - nil))]))))} + (= :error status)) + errors)) + "bg-red-50 text-red-300" + "bg-yellow-100 text-yellow-600")} + svg/alert] + [:template {:x-ref "tt"} + [:ul + (for [[m] errors] + [:li m])]]] + :else + nil)) + (com/icon-button {:type "button" + :color :danger-light + :title (str "Remove ledger entry " entry-id) + :aria-label (str "Remove ledger entry " entry-id) + :data-remove-entry-id entry-id + "@click.prevent.stop" "removeImportEntryRows($el)"} + svg/trash)])))))} [:div.flex.m-4.flex-row-reverse (com/button {:color :primary} "Import")])]]])))]) @@ -332,6 +342,19 @@ var r = await c[0].getType('text/plain') console.log(r) return await r.text() + } + // Removes every row belonging to the same ledger entry (client-source-id) + // as the clicked button. Rows are plain form inputs, so dropping them from + // the DOM drops them from the next import post. + function removeImportEntryRows(el) { + var row = el.closest('tr'); + if (!row) { return; } + var id = row.getAttribute('data-entry-id'); + var body = row.closest('tbody') || row.parentNode; + Array.prototype.slice.call(body.querySelectorAll('tr[data-entry-id]')) + .forEach(function (r) { + if (r.getAttribute('data-entry-id') === id) { r.remove(); } + }); }")] (external-import-form* request) @@ -408,9 +431,6 @@ (html-response (external-import-form* (assoc request :just-parsed? true)))) -(defn line->id [{:keys [source external-id client-code]}] - (str client-code "-" source "-" external-id)) - (defn add-errors [entry all-vendors all-accounts client-locked-lookup all-client-bank-accounts all-client-locations] (let [vendor (all-vendors (:vendor-name entry)) locked-until (client-locked-lookup (:client-code entry)) diff --git a/test/clj/auto_ap/ssr/ledger_test.clj b/test/clj/auto_ap/ssr/ledger_test.clj index 1780f164..34c6234c 100644 --- a/test/clj/auto_ap/ssr/ledger_test.clj +++ b/test/clj/auto_ap/ssr/ledger_test.clj @@ -13,6 +13,7 @@ [clojure.data.csv :as csv] [clojure.string :as str] [datomic.api :as dc] + [hiccup2.core :as hiccup] [malli.core :as mc])) (use-fixtures :each wrap-setup) @@ -619,3 +620,93 @@ (testing "Non-admin cannot bulk-delete" (is (thrown? Exception (sut/bulk-delete {:identity (user-token) :form-params {:selected [1]}}))))) + +;; ============================================================================= +;; External Import - removing an entry from the review grid +;; ============================================================================= + +(defn- import-row [external-id account-code debit credit] + {:external-id external-id + :client-code "REMOVE-TEST" + :source "manual" + :vendor-name "Remove Vendor" + :date (coerce/to-date-time #inst "2021-01-01") + :account-code account-code + :location "HQ" + :debit debit + :credit credit}) + +(defn- entry-rows + "The two rows a balanced ledger entry is typically pasted as." + [external-id debit-account amount] + [(import-row external-id debit-account amount 0.0) + (import-row external-id 2000 0.0 amount)]) + +(deftest external-import-remove-button-test + (testing "Every row is tagged with the entry id the importer groups on" + (let [table (vec (concat (entry-rows "ext-a" 1100 100.0) + (entry-rows "ext-b" 1100 50.0))) + html (str (hiccup/html (sut/external-import-table-form* + {:form-params {:table table} + :form-errors {}})))] + (is (= 2 (count (re-seq #"data-entry-id=\"REMOVE-TEST-manual-ext-a\"" html)))) + (is (= 2 (count (re-seq #"data-entry-id=\"REMOVE-TEST-manual-ext-b\"" html)))) + (testing "and offers a remove button that drops the whole entry" + (is (= 4 (count (re-seq #"removeImportEntryRows\(\$el\)" html)))) + (is (= 2 (count (re-seq #"data-remove-entry-id=\"REMOVE-TEST-manual-ext-a\"" html)))) + (is (= 2 (count (re-seq #"aria-label=\"Remove ledger entry REMOVE-TEST-manual-ext-a\"" html)))))))) + +(deftest external-import-remove-entry-then-import-test + (testing "Removing the failing entry lets the remaining entries import" + (let [_ (setup-test-data [(test-client :db/id "remove-client" + :client/code "REMOVE-TEST" + :client/locations ["HQ"]) + (test-vendor :db/id "remove-vendor" + :vendor/name "Remove Vendor") + {:db/id "remove-account-1100" + :account/numeric-code 1100 + :account/account-set "default" + :account/name "Cash"} + {:db/id "remove-account-2000" + :account/numeric-code 2000 + :account/account-set "default" + :account/name "Accounts Payable"}]) + ;; Three entries, six rows. The middle one posts to an account that + ;; does not exist, which is the kind of error a user has to resolve. + table (vec (concat (entry-rows "ext-good-1" 1100 100.0) + (entry-rows "ext-bad" 99999 75.0) + (entry-rows "ext-good-2" 1100 25.0))) + admin (admin-token) + external-id (fn [id] (str "REMOVE-TEST-manual-" id)) + imported? (fn [db id] + (boolean (dc/q '[:find ?je . + :in $ ?ext + :where [?je :journal-entry/external-id ?ext]] + db (external-id id))))] + + (testing "the bad entry blocks the whole paste" + (is (thrown? Exception (sut/import-ledger {:form-params {:table table} + :identity admin}))) + (let [db (dc/db conn)] + (is (not (imported? db "ext-good-1"))) + (is (not (imported? db "ext-good-2"))))) + + (testing "removing it drops both of its rows" + (let [remaining (vec (remove #(= (external-id "ext-bad") (sut/line->id %)) table))] + (is (= 4 (count remaining))) + + (testing "and the other two entries import" + (let [result (sut/import-ledger {:form-params {:table remaining} + :identity admin}) + db (dc/db conn)] + (is (= 2 (:successful result))) + (is (imported? db "ext-good-1")) + (is (imported? db "ext-good-2")) + (is (not (imported? db "ext-bad"))) + (let [entry (dc/pull db + [:journal-entry/amount + {:journal-entry/line-items [:journal-entry-line/debit + :journal-entry-line/credit]}] + [:journal-entry/external-id (external-id "ext-good-1")])] + (is (= 100.0 (:journal-entry/amount entry))) + (is (= 2 (count (:journal-entry/line-items entry))))))))))))