diff --git a/src/clj/auto_ap/ssr/transaction/edit.clj b/src/clj/auto_ap/ssr/transaction/edit.clj index b23526d7..e66abe69 100644 --- a/src/clj/auto_ap/ssr/transaction/edit.clj +++ b/src/clj/auto_ap/ssr/transaction/edit.clj @@ -883,9 +883,13 @@ #_(require-approval (mut/select-keys (mm/form-schema linear-wizard) #{:transaction/client :transaction/vendor :transaction/memo :transaction/approval-status :db/id})) (mm/form-schema linear-wizard)) - (render-step [this {{:keys [snapshot] :as multi-form-state} :multi-form-state :as request}] + (render-step [this {{:keys [snapshot step-params] :as multi-form-state} :multi-form-state :as request}] (let [tx-id (mm/get-mfs-field multi-form-state :db/id) - tx (d-transactions/get-by-id tx-id)] + tx (d-transactions/get-by-id tx-id) + ;; Preserve explicit mode choice from step-params; only fall back to + ;; row-count heuristic on initial load when no mode has been chosen. + mode (keyword (or (:mode step-params) + (name (manual-mode-initial snapshot))))] (mm/default-render-step linear-wizard this :head [:div.p-2 "Edit Transaction"] @@ -951,7 +955,7 @@ (transaction-rules-view request)] [:div {:x-show "activeForm === 'manual'", :x-transition:enter "transition ease-out duration-500", :x-transition:enter-start "opacity-0 transform scale-95", :x-transition:enter-end "opacity-100 transform scale-100"} [:div {} - (manual-coding-section* (manual-mode-initial snapshot) request) + (manual-coding-section* mode request) (fc/with-field :transaction/approval-status (com/validated-field {:label "Status" @@ -1444,14 +1448,25 @@ amount-mode (or (:amount-mode snapshot) "$") existing-accounts (or (seq (:transaction/accounts step-params)) (seq (:transaction/accounts snapshot))) - default-account (when (and (empty? existing-accounts) vendor-id client-id) + ;; The form always submits an account row (even when empty with account=nil), + ;; so we check if any row has a meaningful account ID. + has-meaningful-accounts? (some #(some? (:transaction-account/account %)) + existing-accounts) + ;; Simple mode: always populate vendor default (overwrite existing). + ;; Advanced mode: populate only when 0 rows OR 1 empty row. + should-populate? (case mode + :simple true + :advanced (or (empty? existing-accounts) + (and (= 1 (count existing-accounts)) + (not has-meaningful-accounts?)))) + default-account (when (and should-populate? vendor-id client-id) (vendor-default-account vendor-id client-id)) render-request - (-> (if (and (empty? existing-accounts) vendor-id client-id) + (-> (if (and should-populate? vendor-id client-id) (let [new-account (cond-> {:db/id (str (java.util.UUID/randomUUID)) :transaction-account/location (or (:account/location default-account) "Shared") :transaction-account/amount (if (= amount-mode "%") 100.0 total)} - default-account (assoc :transaction-account/account (:db/id default-account)))] + default-account (assoc :transaction-account/account (:db/id default-account)))] (-> request (assoc-in [:multi-form-state :snapshot :transaction/accounts] [new-account]) (assoc-in [:multi-form-state :step-params :transaction/accounts] [new-account]))) diff --git a/test/clj/auto_ap/ssr/transaction/edit_simple_advanced_mode_test.clj b/test/clj/auto_ap/ssr/transaction/edit_simple_advanced_mode_test.clj index 117f8c16..cf092feb 100644 --- a/test/clj/auto_ap/ssr/transaction/edit_simple_advanced_mode_test.clj +++ b/test/clj/auto_ap/ssr/transaction/edit_simple_advanced_mode_test.clj @@ -106,7 +106,7 @@ (is (re-find #"Test Account" body) "Response should contain the vendor's default account name"))) - (testing "AC5: vendor selection in simple mode does NOT overwrite already-set account" + (testing "AC5: vendor selection in simple mode DOES overwrite already-set account" (let [result @(dc/transact conn [{:db/id "vendor-id" :vendor/name "Test Vendor"} {:db/id "account-id" @@ -127,9 +127,10 @@ :transaction/client "client-id"}]) tx-id (tempid->id result "transaction-id") vendor-id (tempid->id result "vendor-id") + account-id (tempid->id result "account-id") other-account-id (tempid->id result "other-account-id") client-id (tempid->id result "client-id") - ;; existing-accounts already set means vendor should NOT overwrite + ;; existing-accounts already set — but simple mode should still overwrite existing-accounts [{:db/id "row-id" :transaction-account/account other-account-id :transaction-account/location "DT" @@ -150,12 +151,12 @@ ;; The handler returns an html-response; verify the body is HTML (is (re-find #"manual-coding-section" body) "Response body should contain the manual-coding-section element") - ;; The original account ID must still appear in the rendered HTML - (is (re-find (re-pattern (str other-account-id)) body) - "Response should contain the original (pre-existing) account ID") - ;; The vendor's default account ID must NOT appear — it was not used - (is (not (re-find (re-pattern (str (tempid->id result "account-id"))) body)) - "Response should NOT contain the vendor's default account ID when existing account is set")))) + ;; The vendor's default account SHOULD appear (overwriting previous) + (is (re-find (re-pattern (str account-id)) body) + "Vendor change in simple mode should overwrite with vendor's default account") + ;; The previous account should NOT appear + (is (not (re-find (re-pattern (str other-account-id)) body)) + "Previous account should be replaced by vendor default")))) ;;; --------------------------------------------------------------------------- ;;; AC6: save round-trip — manual mode saves vendor + account to DB @@ -996,3 +997,323 @@ "Vendor name should appear in the HTMX response") (is (re-find (re-pattern (str vendor-id)) (:body response)) "Vendor ID should be preserved in the response HTML"))))) + +;;; --------------------------------------------------------------------------- +;;; Bug: vendor change does not populate account +;;; --------------------------------------------------------------------------- + +(deftest vendor-change-simple-mode-overwrites-test + (testing "BUG: vendor change in simple mode should overwrite existing account" + ;; When a vendor is changed in simple mode, it should always populate + ;; the vendor's default account, even if an account was already set. + (let [result @(dc/transact conn [{:db/id "vendor-id" + :vendor/name "Vendor With Default"} + {:db/id "vendor-account-id" + :account/name "Vendor Default Account" + :account/type :account-type/expense} + {:db/id "vendor-id" + :vendor/default-account "vendor-account-id"} + {:db/id "existing-account-id" + :account/name "Previously Selected Account" + :account/type :account-type/expense} + {:db/id "client-id" + :client/code "VENDORCL" + :client/locations ["DT"]} + {:db/id "transaction-id" + :transaction/amount 100.0 + :transaction/date #inst "2023-01-01" + :transaction/id (str (java.util.UUID/randomUUID)) + :transaction/client "client-id"}]) + tx-id (tempid->id result "transaction-id") + vendor-id (tempid->id result "vendor-id") + vendor-account-id (tempid->id result "vendor-account-id") + existing-account-id (tempid->id result "existing-account-id") + client-id (tempid->id result "client-id") + ;; Simulate form state with an already-selected account (as the form submits) + existing-accounts [{:db/id "row-1" + :transaction-account/account existing-account-id + :transaction-account/location "DT" + :transaction-account/amount 100.0}] + request {:multi-form-state (mm/->MultiStepFormState + {:db/id tx-id + :transaction/client client-id + :transaction/accounts existing-accounts} + [] + {:mode "simple" + :transaction/vendor vendor-id + :transaction/accounts existing-accounts}) + :entity {:db/id tx-id + :transaction/client {:db/id client-id} + :transaction/amount 100.0}} + response (edit-vendor-changed-handler request) + body (:body response)] + ;; The vendor's default account SHOULD appear (overwriting the previous) + (is (re-find (re-pattern (str vendor-account-id)) body) + "BUG: Vendor change in simple mode should overwrite with vendor's default account") + ;; The previously selected account should NOT appear + (is (not (re-find (re-pattern (str existing-account-id)) body)) + "Previously selected account should be replaced by vendor default") + (is (re-find #"Vendor Default Account" body) + "Vendor default account name should appear")))) + +(deftest vendor-change-advanced-mode-empty-row-test + (testing "BUG: vendor change in advanced mode should populate empty row" + ;; In advanced mode with 1 empty row, changing vendor should populate it + (let [result @(dc/transact conn [{:db/id "vendor-id" + :vendor/name "Vendor With Default"} + {:db/id "vendor-account-id" + :account/name "Vendor Default Account" + :account/type :account-type/expense} + {:db/id "vendor-id" + :vendor/default-account "vendor-account-id"} + {:db/id "client-id" + :client/code "ADVEMPTYCL" + :client/locations ["DT"]} + {:db/id "transaction-id" + :transaction/amount 100.0 + :transaction/date #inst "2023-01-01" + :transaction/id (str (java.util.UUID/randomUUID)) + :transaction/client "client-id"}]) + tx-id (tempid->id result "transaction-id") + vendor-id (tempid->id result "vendor-id") + vendor-account-id (tempid->id result "vendor-account-id") + client-id (tempid->id result "client-id") + ;; Simulate advanced mode with 1 empty row (account=nil, as form submits) + empty-row [{:db/id "row-1" + :transaction-account/account nil + :transaction-account/location "Shared" + :transaction-account/amount 100.0}] + request {:multi-form-state (mm/->MultiStepFormState + {:db/id tx-id + :transaction/client client-id + :transaction/accounts empty-row} + [] + {:mode "advanced" + :transaction/vendor vendor-id + :transaction/accounts empty-row}) + :entity {:db/id tx-id + :transaction/client {:db/id client-id} + :transaction/amount 100.0}} + response (edit-vendor-changed-handler request) + body (:body response)] + ;; The vendor's default account SHOULD appear in the row + (is (re-find (re-pattern (str vendor-account-id)) body) + "BUG: Vendor change in advanced mode with empty row should populate it") + (is (re-find #"Vendor Default Account" body) + "Vendor default account name should appear in the row")))) + +(deftest vendor-change-advanced-mode-filled-row-test + (testing "AC15b: vendor change in advanced mode with filled row should NOT overwrite" + ;; In advanced mode with 1 row that already has an account selected, + ;; changing vendor should NOT overwrite it + (let [result @(dc/transact conn [{:db/id "vendor-id" + :vendor/name "Vendor With Default"} + {:db/id "vendor-account-id" + :account/name "Vendor Default Account" + :account/type :account-type/expense} + {:db/id "vendor-id" + :vendor/default-account "vendor-account-id"} + {:db/id "existing-account-id" + :account/name "Manually Selected Account" + :account/type :account-type/expense} + {:db/id "client-id" + :client/code "ADVFILLEDCL" + :client/locations ["DT"]} + {:db/id "transaction-id" + :transaction/amount 100.0 + :transaction/date #inst "2023-01-01" + :transaction/id (str (java.util.UUID/randomUUID)) + :transaction/client "client-id"}]) + tx-id (tempid->id result "transaction-id") + vendor-id (tempid->id result "vendor-id") + vendor-account-id (tempid->id result "vendor-account-id") + existing-account-id (tempid->id result "existing-account-id") + client-id (tempid->id result "client-id") + ;; Advanced mode with 1 row that already has an account + filled-row [{:db/id "row-1" + :transaction-account/account existing-account-id + :transaction-account/location "DT" + :transaction-account/amount 100.0}] + request {:multi-form-state (mm/->MultiStepFormState + {:db/id tx-id + :transaction/client client-id + :transaction/accounts filled-row} + [] + {:mode "advanced" + :transaction/vendor vendor-id + :transaction/accounts filled-row}) + :entity {:db/id tx-id + :transaction/client {:db/id client-id} + :transaction/amount 100.0}} + response (edit-vendor-changed-handler request) + body (:body response)] + ;; The existing account should still be there + (is (re-find (re-pattern (str existing-account-id)) body) + "Existing account should remain when vendor changes in advanced mode with filled row") + ;; The vendor's default account should NOT appear + (is (not (re-find (re-pattern (str vendor-account-id)) body)) + "Vendor default should NOT overwrite filled row in advanced mode")))) + +(deftest vendor-change-advanced-mode-two-rows-test + (testing "AC15c: vendor change in advanced mode with 2+ rows should NOT modify any" + ;; In advanced mode with 2 or more rows, vendor change should not touch any row + (let [result @(dc/transact conn [{:db/id "vendor-id" + :vendor/name "Vendor With Default"} + {:db/id "vendor-account-id" + :account/name "Vendor Default Account" + :account/type :account-type/expense} + {:db/id "vendor-id" + :vendor/default-account "vendor-account-id"} + {:db/id "account-1" + :account/name "Account One" + :account/type :account-type/expense} + {:db/id "account-2" + :account/name "Account Two" + :account/type :account-type/expense} + {:db/id "client-id" + :client/code "ADVTWOROWCL" + :client/locations ["DT"]} + {:db/id "transaction-id" + :transaction/amount 100.0 + :transaction/date #inst "2023-01-01" + :transaction/id (str (java.util.UUID/randomUUID)) + :transaction/client "client-id"}]) + tx-id (tempid->id result "transaction-id") + vendor-id (tempid->id result "vendor-id") + vendor-account-id (tempid->id result "vendor-account-id") + account-1 (tempid->id result "account-1") + account-2 (tempid->id result "account-2") + client-id (tempid->id result "client-id") + ;; Advanced mode with 2 rows + two-rows [{:db/id "row-1" + :transaction-account/account account-1 + :transaction-account/location "DT" + :transaction-account/amount 50.0} + {:db/id "row-2" + :transaction-account/account account-2 + :transaction-account/location "DT" + :transaction-account/amount 50.0}] + request {:multi-form-state (mm/->MultiStepFormState + {:db/id tx-id + :transaction/client client-id + :transaction/accounts two-rows} + [] + {:mode "advanced" + :transaction/vendor vendor-id + :transaction/accounts two-rows}) + :entity {:db/id tx-id + :transaction/client {:db/id client-id} + :transaction/amount 100.0}} + response (edit-vendor-changed-handler request) + body (:body response)] + ;; Both existing accounts should remain + (is (re-find (re-pattern (str account-1)) body) + "First row account should remain") + (is (re-find (re-pattern (str account-2)) body) + "Second row account should remain") + ;; Vendor default should NOT appear + (is (not (re-find (re-pattern (str vendor-account-id)) body)) + "Vendor default should NOT modify rows when 2+ exist")))) + +(deftest vendor-change-client-specific-override-test + (testing "BUG: vendor change should use client-specific account override if present" + ;; When a vendor has a client-specific account override, changing vendor + ;; should populate the client-specific account, not the global default. + (let [result @(dc/transact conn [{:db/id "global-account-id" + :account/name "Global Default" + :account/type :account-type/expense} + {:db/id "client-specific-account-id" + :account/name "Client Specific Account" + :account/type :account-type/expense} + {:db/id "client-id" + :client/code "CLIOVERRIDE" + :client/locations ["DT"]} + {:db/id "vendor-id" + :vendor/name "Clientized Vendor" + :vendor/default-account "global-account-id" + :vendor/account-overrides [{:vendor-account-override/client "client-id" + :vendor-account-override/account "client-specific-account-id"}]}]) + vendor-id (tempid->id result "vendor-id") + client-id (tempid->id result "client-id") + global-account-id (tempid->id result "global-account-id") + client-specific-account-id (tempid->id result "client-specific-account-id") + ;; Simple mode with empty account row + empty-row [{:db/id "row-1" + :transaction-account/account nil + :transaction-account/location "Shared" + :transaction-account/amount 100.0}] + request {:multi-form-state (mm/->MultiStepFormState + {:db/id 999999 + :transaction/client client-id + :transaction/accounts empty-row} + [] + {:mode "simple" + :transaction/vendor vendor-id + :transaction/accounts empty-row}) + :entity {:db/id 999999 + :transaction/client {:db/id client-id} + :transaction/amount 100.0}} + response (edit-vendor-changed-handler request) + body (:body response)] + ;; The client-specific account should appear, not the global default + (is (re-find (re-pattern (str client-specific-account-id)) body) + "BUG: Vendor change should populate client-specific account override") + (is (re-find #"Client Specific Account" body) + "Client-specific account name should appear") + ;; The global default should NOT appear + (is (not (re-find (re-pattern (str global-account-id)) body)) + "Global vendor default should NOT appear when client override exists")))) + +;;; Update AC5: simple mode SHOULD overwrite existing accounts +(deftest vendor-change-simple-mode-overwrites-ac5-test + (testing "AC5 UPDATED: vendor selection in simple mode DOES overwrite already-set account" + (let [result @(dc/transact conn [{:db/id "vendor-id" + :vendor/name "Test Vendor"} + {:db/id "account-id" + :account/name "Test Account" + :account/type :account-type/expense} + {:db/id "vendor-id" + :vendor/default-account "account-id"} + {:db/id "other-account-id" + :account/name "Other Account" + :account/type :account-type/expense} + {:db/id "client-id" + :client/code "TESTCL2" + :client/locations ["DT"]} + {:db/id "transaction-id" + :transaction/amount 100.0 + :transaction/date #inst "2023-01-01" + :transaction/id (str (java.util.UUID/randomUUID)) + :transaction/client "client-id"}]) + tx-id (tempid->id result "transaction-id") + vendor-id (tempid->id result "vendor-id") + account-id (tempid->id result "account-id") + other-account-id (tempid->id result "other-account-id") + client-id (tempid->id result "client-id") + ;; existing-accounts already set — but simple mode should still overwrite + existing-accounts [{:db/id "row-id" + :transaction-account/account other-account-id + :transaction-account/location "DT" + :transaction-account/amount 100.0}] + request {:multi-form-state (mm/->MultiStepFormState + {:db/id tx-id + :transaction/client client-id + :transaction/accounts existing-accounts} + [] + {:mode "simple" + :transaction/vendor vendor-id + :transaction/accounts existing-accounts}) + :entity {:db/id tx-id + :transaction/client {:db/id client-id} + :transaction/amount 100.0}} + response (edit-vendor-changed-handler request) + body (:body response)] + ;; The handler returns an html-response; verify the body is HTML + (is (re-find #"manual-coding-section" body) + "Response body should contain the manual-coding-section element") + ;; The vendor's default account SHOULD appear (overwriting previous) + (is (re-find (re-pattern (str account-id)) body) + "BUG: Vendor change in simple mode should overwrite with vendor's default account") + ;; The previous account should NOT appear + (is (not (re-find (re-pattern (str other-account-id)) body)) + "Previous account should be replaced by vendor default"))))