The function was 199 lines, dominated by a 102-line update_details closure.
Extract the two party-mapping branches into module helpers
_apply_purchase_party_details / _apply_sales_party_details and the address
lookup into _get_linked_address; update_details is now a 6-line dispatcher.
make_inter_company_transaction drops to ~104 lines. No behaviour change
(inter-company SI->PI and PO->SO suites green).
* fix(coa_importer): allow importing COA only for `Accounts Manager`
Co-authored-by: Pratheep S <pratheeps2024@gmail.com>
* fix(coa_importer): check permissions in `unset_existing_data`
---------
Co-authored-by: Pratheep S <pratheeps2024@gmail.com>
Convert raw `frappe.db.sql` in the Setup, Utilities, Templates and Regional
areas to `frappe.qb` / the ORM so the same code runs on MariaDB and Postgres.
Behaviour is preserved on MariaDB; the conversions also make these paths valid
under Postgres' stricter SQL (GROUP BY, case-sensitivity, reserved words).
Conversions of note (behaviour kept identical to the MariaDB original):
- email_digest: ToDo ordering replicated with a CASE that mirrors MySQL
`field(priority,'High','Medium','Low')` (unknown/NULL -> 0, sorts first),
NULL-date-first and a `name` tie-break for a deterministic LIMIT.
- company.get_all_transactions_annual_history: the cross-DocType UNION + GROUP BY
is replaced by one grouped query per DocType merged with a Counter, so two
different DocTypes sharing a transaction_date still collapse into one bucket.
- templates/utils.send_message: contact lookup wraps both sides in LOWER() to
keep MariaDB's case-insensitive email match on case-sensitive Postgres.
- regional/irs_1099 & uae_vat_201: address ranking and emirate aggregation
rebuilt with CASE/aggregate selects that satisfy Postgres GROUP BY, with a
deterministic tie-break on the LIMIT-1 address lookups.
- utilities/product.get_item_codes_by_attributes: numeric attribute values are
cast with cstr() so Postgres doesn't reject `varchar = numeric`.
Tests (run on both MariaDB and Postgres, --lightmode):
- New: company merge test, authorization_rule duplicate-check, youtube report,
templates/utils, and utilities/templates page reports (partners, rfq,
material_request_info, product, utilities __init__).
- Existing suites kept green: company, email_digest, transaction_deletion_record,
irs_1099, uae_vat_201.
Deferred (tracked separately):
- setup/doctype/authorization_control.py still has raw `.format()` SELECTs;
left for its own PR.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- 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>
The second Project in test_sales_order_link_is_not_overwritten_by_second_project
was inserted without a company, which only succeeds when a default company is
configured. On a fresh CI site this raised MandatoryError. Set company from the
sales order explicitly.
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>
