- Filter the parent Maintenance Visit's docstatus (mv.docstatus != 2) via a qb join, as
the original SQL did, instead of the child Maintenance Visit Purpose row's docstatus.
Synced in normal flows, but exactly faithful to the original intent.
- Add a limit(500) to bound the read on a cancellation path.
Adds two both-engine tests calling on_cancel directly: an active (non-cancelled) visit
blocks the claim cancel; with no referencing visit the claim is marked Cancelled.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert the remaining raw frappe.db.sql in the Support module to frappe.qb / ORM so the
queries run on PostgreSQL as well as MariaDB. Faithful conversions -- no MariaDB
behaviour change:
- issue.py, warranty_claim.py (Maintenance Visit lookup / make_maintenance_visit)
- reports: first_response_time_for_issues and issue_summary (GROUP BY on the grouped
Date(creation)/based-on field + Avg/Count -- Postgres-valid), support_hour_distribution
Tests: existing issue suite (35) passes on both engines; adds both-engine tests for the
previously-untested warranty_claim mapper (3) and the three reports. All green on
MariaDB and PostgreSQL.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BootStrapTestData.make_workstation() created _Test Workstation 1 referencing the
warehouse "_Test warehouse - _TC" (lowercase 'w'), but make_warehouse() never
creates that name -- it makes "_Test Warehouse - _TC" (capital W) and others.
On MariaDB the lowercase reference resolves to the capital warehouse via the default
case-insensitive collation, so it silently works. On PostgreSQL (case-sensitive) the
link cannot be found, so on a freshly-bootstrapped site make_workstation() raises
LinkValidationError: Could not find Warehouse: _Test warehouse - _TC, which aborts the
module-level BootStrapTestData() import and blocks every test extending ERPNextTestSuite.
It was masked only on sites where the workstation was already bootstrapped (make_records
skips existing) or where the lowercase name happened to exist from legacy data.
Point the fixture at the existing "_Test Warehouse - _TC". Verified on a freshly-reset
site: the buggy fixture raises the LinkValidationError on Postgres and passes after the
fix; MariaDB passes either way.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
set_pos_fields was a 97-line method (cyclomatic complexity E/36). Split
it into a small orchestrator plus focused helpers, each preserving the
exact for_validate semantics (A/5 after).
Collapse the three near-identical mode-of-payment query builders onto a
shared _enabled_mode_of_payment_query, and add type hints and docstrings.
Public signatures and return shapes are unchanged.
Pin the behaviour of POSService.set_pos_fields (POS-profile default
resolution and the for_validate guard) and the mode-of-payment query
helpers before refactoring them.
- Fix N+1: the per-lead conversion issued 4 queries per lead (Opportunity, Quotation,
Issue, Communication). Collect the reference documents for all leads in 3 bulk
queries, then one Communication query per lead -> ~N+3 round-trips instead of 4N,
matching the original single-query-per-lead cost.
- Constrain reference_doctype in the Communication lookup (names are unique only within
a doctype), closing a latent cross-doctype name-collision gap the original also had.
Both-engine test still green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert the remaining raw frappe.db.sql in the CRM module to frappe.qb / ORM so the
queries run on PostgreSQL as well as MariaDB. Faithful conversions -- no MariaDB
behaviour change:
- opportunity.py, doctype/utils.py (get_last_interaction)
- reports: campaign_efficiency, first_response_time_for_opportunity (GROUP BY on the
grouped Date(creation) + Avg -- Postgres-valid), lead_conversion_time,
prospects_engaged_but_not_converted
lead_conversion_time also keeps the IS NOT NULL communication-date guard (forward-port
of the fix already on develop). Also drops invalid backtick notation from two get_all
order_by clauses in doctype/utils.py (order_by="`creation` DESC"), which frappe's
query engine rejects -- a latent failure on both engines, surfaced by the new test.
Tests: existing opportunity suite plus new both-engine tests for the four previously
untested reports/utils. All green on MariaDB and PostgreSQL.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- cancel_movement_entries: filter the parent Asset Movement's docstatus via a qb join
(as the original SQL did), instead of the child Asset Movement Item.docstatus. Behaviour
is identical in normal flows (child docstatus is synced) but this is exactly faithful.
- get_maintenance_log: add a both-engine test for this previously-untested whitelisted
endpoint. Confirms the frappe v16 dict aggregate field spec ({"COUNT": ...}) runs and
returns correct per-status counts (no runtime crash).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Convert the remaining raw frappe.db.sql in the Assets module to frappe.qb / ORM so
the queries run on PostgreSQL as well as MariaDB. Faithful 1:1 conversions -- no
MariaDB behaviour change:
- asset.py (gl-entry / bom-cost fetches), asset_maintenance.py (team members),
asset_movement.py (latest location/custodian), location.py (get_children)
- fixed_asset_register.py: the depreciation-amount aggregate groups by asset.name
(the primary key) selecting only asset.name + Sum(gle.debit), which is valid under
Postgres strict GROUP BY (PK functional dependency)
Tests: existing asset (61), asset_maintenance, asset_movement and location suites
pass on both engines; adds a test for the previously-untested Fixed Asset Register
report (covers the GROUP BY aggregate on both engines).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review (#56113): add a customer filter to the is_pos test so the
customer-subquery fix is covered (a.customer was unselected before and errored).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review / semgrep (#56112): convert the last raw f-string query
(budget_records, with a dynamic dimension column + tree EXISTS condition) to
frappe.qb, clearing the sql-format-injection finding. Also switch the two
remaining implicit comma-joins (get_requested_amount / get_ordered_amount) to
explicit .join().on() for consistency. test_budget 23/23 on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review (#56111):
- pos_closing get_payments grouped by mode_of_payment but selected a bare account ->
Postgres GroupingError. Wrap in Max() (deterministic, both engines agree; account is
consumed downstream for the change-amount adjustment). test_pos_closing_entry 9/9 both engines.
- get_voucherwise_gl_entries: add limit=0 to make the unbounded fetch explicit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review (#56105): when there's no matching communication, first_contact is
None and date_diff(invoice_date, None) treats None as today, giving a wrong
(negative) duration. Skip the entry instead, mirroring the communication_count guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Converts the report's raw f-string SQL (incl. 3-way UNIONs) to frappe.qb. Split out of #56082.
The non-POS path is MariaDB-identical. get_pos_invoice_data had a loose GROUP BY that
errors on Postgres; line-level warehouse/cost_center/mode_of_payment are now Max()
(deterministic, both engines agree), the unused item_code dropped, and customer added to
the invoice subquery so the customer filter works (it referenced a column the subquery
never selected). + a test covering the previously-untested is_pos path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Converts budget.py raw SQL to frappe.qb for Postgres compatibility. Split out of #56082.
Mostly MariaDB-identical, with one behaviour change: get_requested_amount summed
Sum(qty) * <arbitrary rate> (a bare rate column, invalid on Postgres and wrong when an
item was requested at different rates). Now Sum(qty * rate) per line -- correct, and
identical on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure MariaDB-identical conversion for Postgres compatibility. Split out of #56082.
bank_clearance, pos_closing_entry, process_statement_of_accounts, party, utils, and
sales_invoice/services/pos converted to frappe.qb; bundles erpnext/utilities/query.py
(the get_match_conditions_qb helper, frappe#40075 follow-up) which
process_statement_of_accounts depends on. No behaviour change on MariaDB.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure MariaDB-identical conversion (raw frappe.db.sql -> frappe.qb / portable
functions) for Postgres compatibility. Split out of #56082.
general_ledger, gl_entry, gl_validator, period_closing_voucher, deferred_revenue,
process_payment_reconciliation + their tests. No behaviour change on MariaDB;
verified equivalent and the suites pass on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review (#56105): the IS NOT NULL guard can return no rows (the count above
filters on sender, this query on recipients), so [0][0] would raise IndexError.
Fall back to None when empty, matching the prior behaviour for that case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review (#56101): with 3+ orders the proportional shares could drift by a
sub-cent and not sum back to the grand total, leaving the last payment term
"Partly Paid". The last order (sorted, so MariaDB and Postgres agree) now absorbs
the residual: grand_total - sum(prior shares). Extended the test to three orders.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Parity sweep findings in queries already merged in develop:
- pos_invoice.py: serial_no return-match matched case-sensitively on Postgres (case-insensitive
on MariaDB). Replaced the get_all or_filters lookup with a qb query that case-folds both sides
via Lower() (no-op on MariaDB).
- controllers/queries.py + pick_list.py: the Locate()-based relevance ranking in link-query
ORDER BY is case-sensitive on Postgres (Strpos) vs case-insensitive on MariaDB (Locate), so
autocomplete order differed. Lower() both arguments so the ranking matches on both engines.
- crm/lead_conversion_time.py: "first contact" used ORDER BY communication_date LIMIT 1 read by
index; a NULL date sorts first on MariaDB but last on Postgres, changing the result. Added
`communication_date IS NOT NULL` so both engines return the earliest real contact date.
Verified on MariaDB and Postgres: test_pick_list 40/40, test_pos_invoice 26/26 on both engines.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
