get_pick_list_query selects Sales Order.customer (a joined table's column) while
grouping only by Pick List.name. Postgres' functional-dependency relaxation applies
to a table's own primary key, not to a joined table's columns, so the query raises
GroupingError on Postgres. MariaDB arbitrary-picks and runs.
customer is already pinned to a single value by `WHERE Sales Order.customer = filter`,
so adding it to the GROUP BY is identical on MariaDB and valid on Postgres.
Test (errors with GroupingError on the old code on Postgres, passes on both engines):
- test_get_pick_list_query_postgres_valid: a submitted pick list for a customer is
returned by the link query.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
payment_terms_status_for_sales_order grouped invoice rows by `sii.parent` and took
`Max(sii.sales_order)`, so an invoice that bills several Sales Orders was credited
in full to one arbitrary order and the rest were starved of that payment.
get_so_with_invoices now returns one row per (invoice, sales_order) and splits the
invoice's grand total across the orders in proportion to each order's net line
amount on that invoice. A single-order invoice keeps the full grand total (ratio 1),
so the common case is unchanged; the split is pure Python over deterministic SQL,
so MariaDB and Postgres produce identical results (100% parity).
Test (fails on the old code, passes on both engines):
- test_invoice_billing_multiple_orders_splits_proportionally: one invoice billing two
SOs 600/400 -> each order credited its share, summing to the grand total. Old
Max(sales_order) collapsed the invoice onto one order.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Same class as the sub_assembly_queries / bom_stock_analysis fixes in this PR: two
more merged queries wrapped a non-functionally-dependent column in Max(), so the
engines can disagree and the value is wrong for the case the column drives.
bom_explosion._subitems_query — Max(is_phantom_item):
Rows are grouped by item_code and get_subitems() drops any grouped row whose
is_phantom_item is truthy. When one item_code is listed in a BOM both as a
phantom sub-assembly and as a plain raw material, Max() returns 1 and the real
raw material is silently dropped from the plan. Use Min(): an item is phantom
only when EVERY line for it is phantom, so a real material is never lost.
required_items._material_transfer_qty_by_item — Max(original_item):
original_item is the output dict key. The same item B can be transferred both
for itself (original_item NULL) and as a substitute for required item A
(original_item=A). Grouping by item_code alone with Max() merged the two and
credited B's whole transfer to A, leaving B at 0. Group by
(item_code, original_item) and accumulate into the keyed dict so each transfer
is credited to the right required item (two rows can resolve to one key, e.g.
A's own transfer and B-for-A, hence += not plain assignment).
Both were previously undefined SQL (loose GROUP BY); the fix makes MariaDB and
Postgres agree on the correct, deterministic value. Other Max()-wrapped columns
in these queries are functionally dependent on the grouped item and unchanged.
Tests (fail on the old code, pass on both engines):
- test_subitems_query_keeps_real_rm_listed_alongside_phantom
- test_transferred_qty_not_misattributed_between_item_and_its_substitute
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test was mutating an already-submitted RFQ, which raised
UpdateAfterSubmitError because cost_center lacks allow_on_submit.
Use do_not_submit=True, set cost_center on the draft, save, then submit.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Address review on #56090: get_bom_data groups components by item_code, so it
picks one representative BOM Item line for the (bom_no, is_phantom_item) pair.
Taking the first line by idx dropped the phantom flag when a non-phantom line
was listed before the phantom one, so explode_phantom_boms skipped the sub-BOM.
Keep one row per item_code (preserving the qty_per_unit total per component
rather than widening the GROUP BY), but make the representative phantom-
preferring: the first line, upgraded to the first phantom line if any exists.
A phantom sub-BOM is therefore never dropped due to line order, on either engine.
Adds test_phantom_explosion_when_phantom_line_is_not_first (phantom line at
idx 2) alongside the existing idx-1 case; both pass on MariaDB and Postgres and
the new one fails on the naive first-line representative.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a component is listed more than once in a BOM pointing at different
sub-BOMs (e.g. one phantom, one not), two queries grouped the duplicate
lines into a single row and aggregated bom_no and is_phantom_item with
*independent* Max(). That could pair the phantom flag of one line with the
bom_no of another, so the consumer recursed into the wrong sub-BOM:
- sub_assembly_queries._sub_assembly_rm_query keys on (item_code, bom_no)
and recurses on is_phantom_item. An incoherent pair sent raw-material
resolution down the wrong sub-assembly BOM.
- bom_stock_analysis.get_bom_data: explode_phantom_boms recurses into
bom_no only when is_phantom_item is set; an incoherent pair exploded a
non-phantom sub-BOM as if it were phantom (or vice-versa).
Fix:
- sub_assembly_queries: group also by (bom_no, is_phantom_item) so each
distinct sub-BOM is its own coherent row.
- bom_stock_analysis: drop the two independent Max()es and attach a single
representative line (lowest idx) per item_code before exploding.
This was previously undefined SQL (loose GROUP BY); the fix makes MariaDB
and Postgres agree on a deterministic, coherent pairing. Other Max()-wrapped
columns are functionally dependent on the grouped item and keep their value
on both engines.
Tests (fail on the old code, pass on both engines):
- test_phantom_explosion_picks_coherent_sub_bom: duplicate-component BOM
explodes the phantom sub-BOM, not the lexically-larger non-phantom one.
- test_sub_assembly_rm_query_keeps_bom_no_phantom_pair_coherent: the query
returns one coherent row per distinct sub-BOM with the right phantom flag.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_future_payments_from_journal_entry has no GROUP BY: the Sum() makes it an
implicit single-group aggregate, so every future-dated JE payment company-wide
collapses into ONE row keyed by an arbitrary (invoice, party). The Accounts
Receivable report then allocates the entire future sum against that one invoice and
shows zero future payment for every other invoice. This predates the postgres work
(MariaDB returned an arbitrary single row); the qb conversion only made the arbitrary
pick deterministic via Max().
Add an explicit GROUP BY (je.name, jea.reference_name, jea.party, jea.party_type,
je.posting_date, je.cheque_no) and drop the Max() wrappers, so each (JE, invoice,
party) is its own future-payment row -- matching the Payment Entry path and the
(invoice_no, party) keying the report's allocator already expects. Identical on
MariaDB and PostgreSQL.
Ships a JE-path future-payment test (one future JE paying two invoices -> each
invoice keeps its own future amount; fails on the old code with 0 != 50).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
stock_delivered_but_not_billed_gl_entries (C/15) is split into a thin loop
plus _sdbnb_booking_for_item (eligibility + valuation), _is_sdbnb_reversal
(the account predicate) and _append_sdbnb_gl_entries (the two GL rows) —
guard clauses replace the nested continues. get_gle_for_change_amount now
uses the shared _amount_in_account_currency helper. No C-rank functions
remain in the file. Behaviour unchanged; characterization tests and the
full Sales Invoice suite (133) green.
Three Query Reports embedded double-quoted string literals and an unquoted table
identifier that error on PostgreSQL. Portability-only, no behaviour change on either engine:
- material_requests_for_which_supplier_quotations_are_not_created,
requested_items_to_be_transferred: double-quoted string literals ("Stopped",
"Material Transfer") -> single quotes (double quotes are identifiers on postgres, not strings).
- items_to_be_requested: quote the `tabBin` table identifier so postgres doesn't lower-case it.
(received_items_to_be_billed was dropped: it is a Script Report, so its `query` field is dead
code and the fix never reaches the DB.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pin the two least-covered SalesInvoiceGLComposer methods before refactor:
- stock_delivered_but_not_billed_gl_entries: billing a perpetual Delivery
Note via Sales Invoice reverses the SDBNB account into COGS for an equal
amount.
- get_gle_for_change_amount: empty without change, mandatory-account error,
and the debit-to / change-account entry pair.
Extract two cross-cutting helpers used across the GL methods:
- _amount_in_account_currency: the repeated 'base if account is in company
currency else transaction amount' ternary (customer, tax, item, POS and
write-off entries).
- _return_aware_against_voucher: the return/self-outstanding against_voucher
rule duplicated in the customer and POS entries.
Pull the per-item income and discount rows into their own builders and
flatten make_item_gl_entries with a guard clause. Complexity drops:
make_customer C11->B7, make_pos C12->B7, make_item C14->B10,
make_discount C13->B10, make_tax->A3. No behaviour change; entry dicts and
amounts are identical (full Sales Invoice suite green).
stock_delivered_but_not_billed_gl_entries is left for a coverage-first pass
(it is the least-tested GL branch).
get_warehouse could never run: it filtered POS Profile on a non-existent
'user' column (users live in the applicable_for_users child table), and
embedded a Python bool (frappe.session["user"] == "") inside a query
builder predicate, which raises before reaching the database. It also
has no callers. Remove it and the now-unused msgprint import.
3-way merged onto develop (preserving the get_party_bank_account import move).
get_subscription_details passes order_by="" so get_all does not inject the
doctype default sort the raw query never had.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3-way merged onto develop, preserving develop's set_exchange_rate(ref_doc=doc) change.
One portable raw query is intentionally kept (as on the source branch).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
