fixes vendor selection bug

This commit is contained in:
2026-05-30 09:21:39 -07:00
parent 5c2cf8a631
commit e156d8bfd8
4 changed files with 305 additions and 150 deletions

View File

@@ -455,6 +455,93 @@ test.describe('Transaction Edit Vendor Pre-population', () => {
}); });
}); });
// Drives the *real* vendor typeahead the way a user does: open the dropdown,
// click a rendered result. The vendor search is backed by Solr (unavailable in
// tests), so the result option is injected into the typeahead's Alpine
// `elements` instead of being fetched. Everything else -- the dropdown's own
// search input firing a native `change` on blur, the `value = element` click
// handler, the Alpine reactivity, and the HTMX round-trip to
// `edit-vendor-changed` -- runs exactly as in production. This is the flow that
// regressed: a stale native `change` from the search input used to win the race
// and revert the vendor to its previous value.
async function selectVendorViaDropdown(page: any, vendorId: number, vendorName: string) {
const wrapper = page.locator('div[hx-post*="edit-vendor-changed"]').first();
const typeahead = wrapper.locator('div.relative[x-data]').first();
// Open the dropdown (tippy renders the popper into [data-tippy-root]).
await typeahead.locator('a[x-ref="input"]').click();
const search = page.locator('[data-tippy-root] input[x-model="search"]').first();
await search.waitFor({ state: 'visible' });
// Type under the 3-char search threshold so no Solr request fires and clears
// our injected option, while still dirtying the input so it fires a native
// `change` on blur -- the event that used to clobber the selection.
await search.fill('te');
// Inject a clickable result into the typeahead's Alpine state.
await typeahead.evaluate(
(el: HTMLElement, opt: { id: number; label: string }) => {
(window as any).Alpine.$data(el).elements = [{ value: opt.id, label: opt.label }];
},
{ id: vendorId, label: vendorName }
);
// Click the rendered option: fires the search input's native change (stale
// value) AND the synthetic change carrying the new value, then HTMX swaps.
await page.locator('[data-tippy-root] a', { hasText: vendorName }).first().click();
await page.waitForResponse(
(response: any) =>
response.url().includes('/edit-vendor-changed') && response.status() === 200
);
await page.waitForTimeout(500);
}
// Opens the edit modal and activates the Manual tab, waiting on the vendor
// typeahead rather than the account grid (which only exists in advanced mode).
async function openManualVendorSection(page: any, transactionIndex: number) {
await page.goto('/transaction2');
await page.waitForSelector('table tbody tr');
const editButton = page
.locator('button[hx-get*="/transaction2/"][hx-get*="/edit"]')
.nth(transactionIndex);
await editButton.click();
await page.waitForSelector('#modal-holder[x-show="open"]', { state: 'visible' });
await page.waitForSelector('#wizardmodal');
await page.click('button:has-text("Manual")');
await page.waitForSelector('div[hx-post*="edit-vendor-changed"]');
}
test.describe('Transaction Edit Vendor Selection', () => {
test('selecting a vendor from the dropdown updates the displayed vendor', async ({ page }) => {
await openManualVendorSection(page, 3);
const testInfo = await getTestInfo(page);
const vendorId: number = testInfo.accounts.vendor;
await selectVendorViaDropdown(page, vendorId, 'Test Vendor');
// The displayed vendor label must reflect the selection after the HTMX
// round-trip. Before the fix this reverted to blank because a stale
// `change` event submitted the previous vendor and its response won.
const label = page
.locator('div[hx-post*="edit-vendor-changed"] span[x-text="value.label"]')
.first();
await expect(label).toHaveText('Test Vendor');
// The server-rendered hidden input must carry the newly selected vendor id.
const hidden = page
.locator(
'div[hx-post*="edit-vendor-changed"] input[type="hidden"][name="step-params[transaction/vendor]"]'
)
.first();
await expect(hidden).toHaveValue(vendorId.toString());
});
});
test.describe('Transaction Link Date Display', () => { test.describe('Transaction Link Date Display', () => {
test('should show payment date when linking to payment', async ({ page }) => { test('should show payment date when linking to payment', async ({ page }) => {
await openEditModalForTransaction(page, 'Transaction for payment link'); await openEditModalForTransaction(page, 'Transaction for payment link');

View File

@@ -83,7 +83,7 @@
"x-ref" "hidden" "x-ref" "hidden"
:type "hidden" :type "hidden"
":value" "value.value" ":value" "value.value"
:x-init (hiccup/raw (str "$watch('value', v => $dispatch('change')); "))))] :x-init (hiccup/raw (str "$watch('value', v => { $el.value = (v && v.value != null) ? v.value : ''; $nextTick(() => $dispatch('change')); }); "))))]
[:div.flex.w-full.justify-items-stretch [:div.flex.w-full.justify-items-stretch
[:span.flex-grow.text-left {"x-text" "value.label"}] [:span.flex-grow.text-left {"x-text" "value.label"}]
[:div {:class "w-3 h-3 m-1 inline ml-1 justify-self-end text-gray-500 self-center"} [:div {:class "w-3 h-3 m-1 inline ml-1 justify-self-end text-gray-500 self-center"}
@@ -104,9 +104,10 @@
(hh/replace-wildcard ["rounded" "border"] "border-bottom bg-gray-100 rounded-t-lg w-full")) (hh/replace-wildcard ["rounded" "border"] "border-bottom bg-gray-100 rounded-t-lg w-full"))
"x-model" "search" "x-model" "search"
"placeholder" (:placeholder params) "placeholder" (:placeholder params)
"@change.stop" ""
"@keydown.down.prevent" "active ++; active = active >= elements.length - 1 ? elements.length - 1 : active" "@keydown.down.prevent" "active ++; active = active >= elements.length - 1 ? elements.length - 1 : active"
"@keydown.up.prevent" "active --; active = active < 0 ? 0 : active" "@keydown.up.prevent" "active --; active = active < 0 ? 0 : active"
"@keydown.enter.prevent.stop" "tippy.hide(); value = elements.length > 0 ? $data.elements[active] : {'value': '', label: ''}; $refs.input.focus()" "@keydown.enter.prevent.stop" "tippy.hide(); value = elements.length > 0 ? $data.elements[active >= 0 ? active : 0] : {'value': '', label: ''}; $refs.input.focus()"
"x-init" "$el.focus(); $watch('search', s => { if($el.value.length > 2) {fetch(addQueryParam(baseUrl, 'q', s)).then(data=>data.json()).then(data => {elements = data; active=-1; tippy.popperInstance.update()}) }})"}] "x-init" "$el.focus(); $watch('search', s => { if($el.value.length > 2) {fetch(addQueryParam(baseUrl, 'q', s)).then(data=>data.json()).then(data => {elements = data; active=-1; tippy.popperInstance.update()}) }})"}]
[:div.dropdown-options {:class "rounded-b-lg overflow-hidden"} [:div.dropdown-options {:class "rounded-b-lg overflow-hidden"}
[:template {:x-for "(element, index) in elements"} [:template {:x-for "(element, index) in elements"}
@@ -116,7 +117,7 @@
"@mouseover" "active = index" "@mouseover" "active = index"
"@mouseout" "active = -1" "@mouseout" "active = -1"
"@click.prevent" "value = element; tippy.hide(); $refs.input.focus()" "@click.prevent" "value = element; tippy.hide(); setTimeout(() => $refs.input.focus(), 10)"
"x-html" "element.label"}]]] "x-html" "element.label"}]]]
[:template {:x-if "elements.length == 0"} [:template {:x-if "elements.length == 0"}
[:li {:class "px-4 py-2 flex gap-2 items-center outline-0 focus:bg-neutral-100 hover:bg-neutral-100 whitespace-nowrap [&.active]:bg-primary-500 text-gray-800 dark:text-gray-100 text-xs "} [:li {:class "px-4 py-2 flex gap-2 items-center outline-0 focus:bg-neutral-100 hover:bg-neutral-100 whitespace-nowrap [&.active]:bg-primary-500 text-gray-800 dark:text-gray-100 text-xs "}

View File

@@ -514,6 +514,7 @@
:hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-vendor-changed) :hx-post (bidi/path-for ssr-routes/only-routes ::route/edit-vendor-changed)
:hx-target "#manual-coding-section" :hx-target "#manual-coding-section"
:hx-swap "outerHTML" :hx-swap "outerHTML"
:hx-sync "this:replace"
:hx-include "closest form"} :hx-include "closest form"}
(fc/with-field :transaction/vendor (fc/with-field :transaction/vendor
(com/validated-field (com/validated-field
@@ -1429,10 +1430,13 @@
(let [multi-form-state (:multi-form-state request) (let [multi-form-state (:multi-form-state request)
snapshot (:snapshot multi-form-state) snapshot (:snapshot multi-form-state)
step-params (:step-params multi-form-state) step-params (:step-params multi-form-state)
mode (keyword (or (:mode step-params) "simple")) mode (keyword (or (:mode step-params)
(get (:form-params request) "mode")
"simple"))
client-id (or (:transaction/client snapshot) client-id (or (:transaction/client snapshot)
(-> request :entity :transaction/client :db/id)) (-> request :entity :transaction/client :db/id))
vendor-id (or (:transaction/vendor step-params) vendor-id (or (:transaction/vendor step-params)
(->db-id (get step-params "transaction/vendor"))
(:transaction/vendor snapshot)) (:transaction/vendor snapshot))
total (Math/abs (or (-> request :entity :transaction/amount) total (Math/abs (or (-> request :entity :transaction/amount)
(:transaction/amount snapshot) (:transaction/amount snapshot)
@@ -1443,7 +1447,7 @@
default-account (when (and (empty? existing-accounts) vendor-id client-id) default-account (when (and (empty? existing-accounts) vendor-id client-id)
(vendor-default-account vendor-id client-id)) (vendor-default-account vendor-id client-id))
render-request render-request
(if (and (empty? existing-accounts) vendor-id client-id) (-> (if (and (empty? existing-accounts) vendor-id client-id)
(let [new-account (cond-> {:db/id (str (java.util.UUID/randomUUID)) (let [new-account (cond-> {:db/id (str (java.util.UUID/randomUUID))
:transaction-account/location (or (:account/location default-account) "Shared") :transaction-account/location (or (:account/location default-account) "Shared")
:transaction-account/amount (if (= amount-mode "%") 100.0 total)} :transaction-account/amount (if (= amount-mode "%") 100.0 total)}
@@ -1451,7 +1455,8 @@
(-> request (-> request
(assoc-in [:multi-form-state :snapshot :transaction/accounts] [new-account]) (assoc-in [:multi-form-state :snapshot :transaction/accounts] [new-account])
(assoc-in [:multi-form-state :step-params :transaction/accounts] [new-account]))) (assoc-in [:multi-form-state :step-params :transaction/accounts] [new-account])))
request)] request)
(assoc-in [:multi-form-state :step-params :transaction/vendor] vendor-id))]
(html-response (html-response
(fc/start-form (:multi-form-state render-request) nil (fc/start-form (:multi-form-state render-request) nil
(fc/with-field :step-params (fc/with-field :step-params

View File

@@ -5,7 +5,8 @@
[auto-ap.solr] [auto-ap.solr]
[auto-ap.ssr.components.multi-modal :as mm] [auto-ap.ssr.components.multi-modal :as mm]
[auto-ap.ssr.form-cursor :as fc] [auto-ap.ssr.form-cursor :as fc]
[auto-ap.ssr.transaction.edit :refer [clientize-vendor [auto-ap.ssr.transaction.edit
:refer [clientize-vendor
edit-vendor-changed-handler edit-vendor-changed-handler
edit-wizard-toggle-mode-handler edit-wizard-toggle-mode-handler
location-select* location-select*
@@ -934,3 +935,64 @@
;; Should NOT show 'Switch to simple mode' ;; Should NOT show 'Switch to simple mode'
(is (not (re-find #"Switch to simple mode" html)) (is (not (re-find #"Switch to simple mode" html))
"AC20: Simple mode should NOT show 'Switch to simple mode' link")))) "AC20: Simple mode should NOT show 'Switch to simple mode' link"))))
;;; ---------------------------------------------------------------------------
;;; Bug: vendor selection gets erased on vendor-changed HTMX response
;;; ---------------------------------------------------------------------------
(deftest vendor-selection-preserved-in-htmx-response-test
(testing "BUG: vendor selection should be preserved when HTMX re-renders the edit form"
(let [result @(dc/transact conn [{:db/id "vendor-id"
:vendor/name "Test Vendor"}
{:db/id "account-id"
:account/name "Existing 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")
account-id (tempid->id result "account-id")
client-id (tempid->id result "client-id")
;; Simulate the request after middleware decoding.
;; In production, form values arrive as strings. The middleware decodes
;; step-params with keyword keys but leaves values as strings.
existing-accounts [{:db/id "row-1"
:transaction-account/account 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"
;; This is how the vendor ID arrives from the form:
;; as a string, not a long.
:transaction/vendor (str vendor-id)
:transaction/accounts existing-accounts})
:entity {:db/id tx-id
:transaction/client {:db/id client-id}
:transaction/amount 100.0}}
;; The handler should return a successful response with the vendor
;; preserved. Currently it crashes because the string vendor-id is
;; not converted to a long before being passed to Datomic.
response (try
(edit-vendor-changed-handler request)
(catch Exception e
{:error e}))]
(is (not (:error response))
(str "BUG: String vendor-id from form submission should be converted to long. "
"Server crashes with: " (some-> response :error ex-message)))
(when-not (:error response)
(is (= 200 (:status response))
"Response should be successful")
(is (re-find #"Test Vendor" (:body response))
"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")))))