From 4fca49bff01b595e64277b2f891a448bd30832be Mon Sep 17 00:00:00 2001 From: Bryce Date: Tue, 26 May 2026 23:20:31 -0700 Subject: [PATCH] fixes a number of issues --- e2e/bulk-code-transactions.spec.ts | 75 ++++++++++++++-- e2e/transaction-edit.spec.ts | 86 +++++++++++++++++-- e2e/transaction-navigation.spec.ts | 6 +- src/clj/auto_ap/ssr/form_cursor.clj | 1 - src/clj/auto_ap/ssr/transaction/bulk_code.clj | 74 ++++++++-------- test/clj/auto_ap/test_server.clj | 20 +++-- 6 files changed, 199 insertions(+), 63 deletions(-) diff --git a/e2e/bulk-code-transactions.spec.ts b/e2e/bulk-code-transactions.spec.ts index c46e585d..d8799224 100644 --- a/e2e/bulk-code-transactions.spec.ts +++ b/e2e/bulk-code-transactions.spec.ts @@ -234,7 +234,7 @@ test.describe('Bulk Code Transactions - Happy Path', () => { await openBulkCodeModal(page); // Should show all transactions - await expect(page.locator('text=Bulk editing 5 transactions')).toBeVisible(); + await expect(page.locator('text=Bulk editing 6 transactions')).toBeVisible(); // Add account at 100% await addNewAccount(page); @@ -263,6 +263,61 @@ test.describe('Bulk Code Transactions - Validation', () => { await expect(page.locator('#modal-holder[x-show="open"]')).toBeVisible(); }); + test('should preserve vendor and status on validation error', async ({ page }) => { + await navigateToTransactions(page); + await selectTransactionByIndex(page, 0); + await openBulkCodeModal(page); + + // Select vendor + const testInfo = await getTestInfo(page); + const vendorId = testInfo.accounts.vendor; + + const vendorContainer = page.locator('div[hx-post*="vendor-changed"]').first(); + const vendorHidden = vendorContainer.locator('input[type="hidden"]').first(); + + await vendorHidden.evaluate((el: HTMLInputElement, value: string) => { + const newInput = document.createElement('input'); + newInput.type = 'hidden'; + newInput.name = el.name; + newInput.value = value; + el.parentNode.replaceChild(newInput, el); + }, vendorId.toString()); + + await vendorContainer.evaluate((el: HTMLElement) => { + el.dispatchEvent(new Event('change', { bubbles: true })); + }); + + await page.waitForResponse(response => response.url().includes('/vendor-changed') && response.status() === 200); + await page.waitForTimeout(500); + + // Select approval status + const statusSelect = page.locator('select[name="step-params[approval-status]"]').first(); + await statusSelect.selectOption('approved'); + + // Vendor selection pre-populated a default account row at 100%. + // Modify its percentage to 50% (invalid - doesn't total 100%). + await setAccountPercentage(page, 0, '50'); + + await submitBulkCodeForm(page); + await page.waitForTimeout(1000); + + // Modal should still be open + await expect(page.locator('#modal-holder[x-show="open"]')).toBeVisible(); + + // Vendor should still be selected + const vendorHiddenAfter = page.locator('input[type="hidden"][name="step-params[vendor]"]').first(); + const vendorValueAfter = await vendorHiddenAfter.inputValue(); + expect(vendorValueAfter).toBe(vendorId.toString()); + + // Status should still be selected + const statusValueAfter = await statusSelect.inputValue(); + expect(statusValueAfter).toBe('approved'); + + // Should show validation error + const errorText = await getModalErrorText(page); + expect(errorText).toContain('does not equal 100%'); + }); + test('should reject when account percentages total less than 100%', async ({ page }) => { await navigateToTransactions(page); await selectTransactionByIndex(page, 0); @@ -447,7 +502,7 @@ test.describe('Bulk Code Transactions - Vendor Pre-population', () => { await page.waitForSelector('table tbody tr'); }); - test('should NOT pre-populate default account when user has multiple clients', async ({ page }) => { + test('should pre-populate non-clientized default account when user has multiple clients', async ({ page }) => { // Switch to multi-client mode await page.request.get('/test-set-client-mode?mode=multi-client'); @@ -480,13 +535,15 @@ test.describe('Bulk Code Transactions - Vendor Pre-population', () => { await page.waitForResponse(response => response.url().includes('/vendor-changed') && response.status() === 200); await page.waitForTimeout(500); - // Should NOT have pre-populated account rows - only the "New account" button row - const accountRows = page.locator('#account-entries tbody tr'); - const rowCount = await accountRows.count(); - - // With multi-client, no pre-population should happen, so only 1 row (the "New account" button) - expect(rowCount).toBe(1); - + // Should pre-populate the vendor's default account (non-clientized) plus the "New account" row + const accountInputs = page.locator('#account-entries input[type="hidden"][name*="[account]"]'); + const accountInputCount = await accountInputs.count(); + expect(accountInputCount).toBe(1); + + // The pre-populated account should be the vendor's raw default account (test-account) + const accountValue = await accountInputs.first().inputValue(); + expect(accountValue).toBe(testInfo.accounts['test-account'].toString()); + // Switch back to single-client mode for other tests await page.request.get('/test-set-client-mode?mode=single-client'); }); diff --git a/e2e/transaction-edit.spec.ts b/e2e/transaction-edit.spec.ts index c9a1bcd0..0ce1b3f4 100644 --- a/e2e/transaction-edit.spec.ts +++ b/e2e/transaction-edit.spec.ts @@ -15,13 +15,8 @@ async function openEditModal(page: any, transactionIndex: number = 0) { await page.waitForSelector('#modal-holder[x-show="open"]', { state: 'visible' }); await page.waitForSelector('#wizardmodal'); - // Click Next to go to the links step (button says "Transaction Actions") - await page.click('button:has-text("Transaction Actions")'); - - // Wait for the links step to load - await page.waitForSelector('text=Transaction Actions', { state: 'visible' }); - - // Click on "Manual" tab + // The modal is now single-page (Edit Transaction). Click "Manual" tab to ensure + // the manual account coding form is active. await page.click('button:has-text("Manual")'); // Wait for the manual form to appear @@ -383,6 +378,83 @@ async function openEditModalForTransaction(page: any, description: string) { await page.waitForSelector('text=Transaction Actions', { state: 'visible' }); } +async function selectVendorFromTypeahead(page: any, vendorName: string) { + const testInfo = await getTestInfo(page); + const vendorId = testInfo.accounts.vendor; + + if (!vendorId) { + throw new Error(`Could not find vendor with name ${vendorName}`); + } + + const vendorContainer = page.locator('div[hx-post*="edit-vendor-changed"]').first(); + const vendorHidden = vendorContainer.locator('input[type="hidden"]').first(); + + await vendorHidden.evaluate((el: HTMLInputElement, value: string) => { + const newInput = document.createElement('input'); + newInput.type = 'hidden'; + newInput.name = el.name; + newInput.value = value; + el.parentNode.replaceChild(newInput, el); + }, vendorId.toString()); + + await vendorContainer.evaluate((el: HTMLElement) => { + el.dispatchEvent(new Event('change', { bubbles: true })); + }); + + await page.waitForResponse(response => response.url().includes('/edit-vendor-changed') && response.status() === 200); + await page.waitForTimeout(500); +} + +test.describe('Transaction Edit Vendor Pre-population', () => { + test('should start with no account rows when transaction has no accounts', async ({ page }) => { + await openEditModal(page, 3); + + await page.click('button:has-text("Manual")'); + await page.waitForSelector('#account-grid-body'); + + // Remove any existing accounts from previous tests + await removeAllAccounts(page); + await page.waitForTimeout(1000); + + const accountRows = page.locator('#account-grid-body tbody tr.account-row'); + const rowCount = await accountRows.count(); + + expect(rowCount).toBe(0); + }); + + test('should pre-populate default account when vendor is selected', async ({ page }) => { + await openEditModal(page, 3); + + await page.click('button:has-text("Manual")'); + await page.waitForSelector('#account-grid-body'); + + // Remove any existing accounts from previous tests + await removeAllAccounts(page); + await page.waitForTimeout(1000); + + const accountRows = page.locator('#account-grid-body tbody tr.account-row'); + const initialRowCount = await accountRows.count(); + expect(initialRowCount).toBe(0); + + await selectVendorFromTypeahead(page, 'Test Vendor'); + + const rowsAfterVendor = page.locator('#account-grid-body tbody tr.account-row'); + const rowCountAfter = await rowsAfterVendor.count(); + + expect(rowCountAfter).toBe(1); + + const accountHidden = page.locator('input[type="hidden"][name*="transaction-account/account"]').first(); + const accountValue = await accountHidden.inputValue(); + + const testInfo = await getTestInfo(page); + expect(accountValue).toBe(testInfo.accounts['test-account'].toString()); + + const amountInput = page.locator('.account-amount-field').first(); + const amountValue = await amountInput.inputValue(); + expect(parseFloat(amountValue)).toBeCloseTo(400.0, 1); + }); +}); + test.describe('Transaction Link Date Display', () => { test('should show payment date when linking to payment', async ({ page }) => { await openEditModalForTransaction(page, 'Transaction for payment link'); diff --git a/e2e/transaction-navigation.spec.ts b/e2e/transaction-navigation.spec.ts index 49df1eec..f0a2a5a5 100644 --- a/e2e/transaction-navigation.spec.ts +++ b/e2e/transaction-navigation.spec.ts @@ -74,17 +74,17 @@ test.describe('Transaction Navigation - Amount Filter Persistence', () => { test('should persist amount filter when navigating to Client Review', async ({ page }) => { // Step 1: Navigate to All page and set amount filter await navigateToTransactions(page, '/transaction2'); - await setAmountFilter(page, '', '250'); + await setAmountFilter(page, '', '500'); // Step 2: Wait for URL to update - await page.waitForURL(url => url.search.includes('amount-lte=250'), { timeout: 5000 }); + await page.waitForURL(url => url.search.includes('amount-lte=500'), { timeout: 5000 }); // Step 3: Click Client Review nav link await clickTransactionNavLink(page, 'Client Review'); // Step 4: Verify filter persisted const feedbackUrl = page.url(); - expect(feedbackUrl).toContain('amount-lte=250'); + expect(feedbackUrl).toContain('amount-lte=500'); }); }); diff --git a/src/clj/auto_ap/ssr/form_cursor.clj b/src/clj/auto_ap/ssr/form_cursor.clj index 82f4d785..6b65879b 100644 --- a/src/clj/auto_ap/ssr/form_cursor.clj +++ b/src/clj/auto_ap/ssr/form_cursor.clj @@ -56,7 +56,6 @@ (defn field-errors ([] - (println "CURRENT IS" *current*) (field-errors *current*)) ([cursor] (get-in *form-errors* (cursor/path cursor)))) diff --git a/src/clj/auto_ap/ssr/transaction/bulk_code.clj b/src/clj/auto_ap/ssr/transaction/bulk_code.clj index 4cc1ea2b..6191a5c3 100644 --- a/src/clj/auto_ap/ssr/transaction/bulk_code.clj +++ b/src/clj/auto_ap/ssr/transaction/bulk_code.clj @@ -185,25 +185,28 @@ :hx-target "#account-entries" :hx-swap "innerHTML" :hx-include "closest form"} - (fc/with-field :vendor - (com/validated-field {:label "Vendor" - :errors (fc/field-errors)} - (com/typeahead {:name (fc/field-name) - :placeholder "Search for vendor..." - :url (bidi/path-for ssr-routes/only-routes :vendor-search) - :content-fn (fn [c] (pull-attr (dc/db conn) :vendor/name c))})))] + (fc/with-field :vendor + (com/validated-field {:label "Vendor" + :errors (fc/field-errors)} + (com/typeahead {:name (fc/field-name) + :placeholder "Search for vendor..." + :url (bidi/path-for ssr-routes/only-routes :vendor-search) + :value (fc/field-value) + :content-fn (fn [c] (pull-attr (dc/db conn) :vendor/name c))})))] ;; Status field - [:div - (fc/with-field :approval-status - (com/validated-field {:label "Status" - :errors (fc/field-errors)} - (com/select {:name (fc/field-name) - :options [["" "No Change"] - ["approved" "Approved"] - ["unapproved" "Unapproved"] - ["suppressed" "Suppressed"] - ["requires_feedback" "Requires Feedback"]]})))] + [:div + (fc/with-field :approval-status + (com/validated-field {:label "Status" + :errors (fc/field-errors)} + (com/select {:name (fc/field-name) + :value (some-> (fc/field-value) + name) + :options [["" "No Change"] + ["approved" "Approved"] + ["unapproved" "Unapproved"] + ["suppressed" "Suppressed"] + ["requires_feedback" "Requires Feedback"]]})))] ;; Accounts section [:div.col-span-2.pt-4 @@ -343,25 +346,26 @@ :percentage 1.0}) (defn- render-accounts-section [request] - (let [step-params (:step-params (:multi-form-state request))] + (let [multi-form-state (:multi-form-state request)] (html-response [:div - (fc/start-form step-params + (fc/start-form multi-form-state (when (:form-errors request) {:step-params (:form-errors request)}) - (fc/with-field :accounts - (com/validated-field - {:errors (fc/field-errors)} - (com/data-grid {:headers [(com/data-grid-header {} "Account") - (com/data-grid-header {:class "w-32"} "Location") - (com/data-grid-header {:class "w-16"} "%") - (com/data-grid-header {:class "w-16"})]} - (fc/cursor-map #(transaction-account-row* {:value %})) - (com/data-grid-new-row {:colspan 4 - :hx-get (bidi/path-for ssr-routes/only-routes - ::route/bulk-code-new-account) - :row-offset 0 - :index (count (fc/field-value))} - "New account")))))]))) + (fc/with-field :step-params + (fc/with-field :accounts + (com/validated-field + {:errors (fc/field-errors)} + (com/data-grid {:headers [(com/data-grid-header {} "Account") + (com/data-grid-header {:class "w-32"} "Location") + (com/data-grid-header {:class "w-16"} "%") + (com/data-grid-header {:class "w-16"})]} + (fc/cursor-map #(transaction-account-row* {:value %})) + (com/data-grid-new-row {:colspan 4 + :hx-get (bidi/path-for ssr-routes/only-routes + ::route/bulk-code-new-account) + :row-offset 0 + :index (count (fc/field-value))} + "New account"))))))]))) (defn- single-client-id [request] "Returns the client ID if the user has access to exactly one client, nil otherwise." @@ -373,10 +377,8 @@ step-params (:step-params (:multi-form-state request)) client-id (single-client-id request) vendor-id (or (:vendor step-params) (:vendor snapshot)) - _ (println ::VENDOR-CHANGED :client-id client-id :vendor-id vendor-id :accounts-empty (empty? (:accounts step-params))) updated-step-params (if (and (empty? (:accounts step-params)) - vendor-id - client-id) + vendor-id) (if-let [default-account (vendor-default-account vendor-id client-id)] (assoc step-params :accounts [(build-default-account-row default-account)]) step-params) diff --git a/test/clj/auto_ap/test_server.clj b/test/clj/auto_ap/test_server.clj index d2639df2..b944de17 100644 --- a/test/clj/auto_ap/test_server.clj +++ b/test/clj/auto_ap/test_server.clj @@ -135,13 +135,19 @@ :payment/status :payment-status/pending :payment/date #inst "2023-06-15") ;; Transaction and unpaid invoice for link testing - (test-transaction :db/id "transaction-id-unpaid" - :transaction/client "client-id" - :transaction/bank-account "bank-account-id" - :transaction/amount -150.0 - :transaction/description-original "Transaction for unpaid invoice link" - :transaction/approval-status :transaction-approval-status/unapproved) - (test-invoice :db/id "invoice-unpaid-id" + (test-transaction :db/id "transaction-id-unpaid" + :transaction/client "client-id" + :transaction/bank-account "bank-account-id" + :transaction/amount -150.0 + :transaction/description-original "Transaction for unpaid invoice link" + :transaction/approval-status :transaction-approval-status/unapproved) + (test-transaction :db/id "transaction-id-feedback" + :transaction/client "client-id" + :transaction/bank-account "bank-account-id" + :transaction/amount 400.0 + :transaction/description-original "Transaction for feedback review" + :transaction/approval-status :transaction-approval-status/requires-feedback) + (test-invoice :db/id "invoice-unpaid-id" :invoice/client "client-id" :invoice/vendor "vendor-id" :invoice/total 150.0