get_serialized_items had zero callers anywhere (Python, JS, run_method) on develop
and after the refactor; it was relocated into SerialBatchBundleService by mistake
instead of being dropped. Delete it — also removes a raw frappe.db.sql_list query
that duplicated the ORM helper get_serial_or_batch_items.
The @frappe.request_cache decorator keyed on `self`, which after the service
extraction is a transient SerialBatchBundleService built per delegated call, so the
request-wide dedup was lost and dead instances were pinned in request_cache. Use
frappe.get_cached_value on the Item instead: caching is keyed by the item (request-
local + redis), effective regardless of service-instance churn, and the redundant
frappe.db.exists query is dropped.
Verified: ledger snapshots + serial and batch bundle suite stay green.
Address code-review findings on the Phase-0 safety net:
- Per-test savepoint/rollback isolation so cumulative SLE fields
(qty_after_transaction, stock_value, valuation_rate) are deterministic
regardless of test order or leftover state (were order-coupled before).
- Backdate prerequisite stock to PREREQUISITE_DATE so balances are positive and
independent of the wall-clock date.
- Capture has_serial_and_batch_bundle (boolean linkage, not the volatile docname)
so a dropped serial/batch bundle link is caught.
- Add pr_batch_item and pr_serial_item scenarios to exercise SerialBatchBundleService
(the largest extraction, previously uncovered).
Goldens regenerated. Verified deterministic across repeated assert runs.
validate_putaway_capacity selected the 'disable' field but checked rule.get('disabled')
(always None), so disabled rules still enforced capacity and could wrongly raise
'Over Receipt'. Use the correct 'disable' key. Pre-existing bug surfaced during the
stock_controller refactor review.
These clusters are really other doctypes' logic parked on StockController, so they
move next to the doctype that owns them rather than into a stock service:
- set_landed_cost_voucher_amount / get_item_account_wise_lcv_entries /
has_landed_cost_amount -> landed_cost_voucher.py (free functions). Controller keeps
thin delegators (called as doc.X from 4 GL composers, buying_controller and the LCV
doctype).
- validate_putaway_capacity -> putaway_rule.py (free function, next to
get_available_putaway_capacity it already used). Controller keeps a delegator
(validate hook + Stock Entry/Reconciliation); prepare_over_receipt_message becomes a
private helper there.
Drops now-unused Sum/defaultdict imports from stock_controller.
Behaviour-preserving: ledger snapshots, putaway and landed-cost suites stay green.
Move internal-transfer warehouse/currency/packed-item/over-receipt-qty validation
into erpnext/stock/services/internal_transfer.py as a delegating service. This is
the stock-side counterpart to accounts/services/internal_transfer.py (party/rate/
pricing). validate_internal_transfer keeps a controller delegator (validate hook);
the other 7 methods are internal-only. The is_internal_transfer() predicate is
already consolidated on AccountsController.
Behaviour-preserving: ledger snapshots + DN/PR internal-transfer suites stay green.
Move quality-inspection validation (validate_inspection + validate_qi_presence/
submission/rejection) into erpnext/stock/services/quality_inspection.py as a
delegating service. validate_inspection keeps a controller delegator (called from
validate() and 3 other doctypes); the three row-level helpers are internal-only.
The whitelisted module fns check_item_quality_inspection / make_quality_inspections
stay in stock_controller (stable endpoint paths).
Behaviour-preserving: ledger snapshots + quality inspection suite stay green.
Relocate get_voucher_details, check_expense_account and get_debit_field_precision
from StockController to BaseStockGLComposer, where they are only used (by compose()
and AssetCapitalizationGLComposer). Call sites flipped from doc.X to self.X.
Inventory-account resolution (get_inventory_account_map/_dict, etc.) stays on the
controller: it is a doc-contract method called as doc.X from non-stock-composer code
(PI controller/composer, accounts/utils, repost_accounting_ledger), so it cannot fold
into BaseStockGLComposer. make_gl_entries / make_gl_entries_on_cancel / add_gl_entry
likewise stay (contract entry points).
Behaviour-preserving: ledger snapshots, subcontracting receipt and asset
capitalization suites stay green.
Move SLE building and reposting (get_sl_entries, update_inventory_dimensions,
get_stock_ledger_details, get_items_and_warehouses, make_sl_entries,
repost_future_sle_and_gle) into erpnext/stock/services/stock_ledger.py as a
delegating service. All six keep thin controller delegators (each has external
callers). The repost helper *functions* stay module-level in stock_controller
(imported widely); the service calls them. Also drop import orphaned by this and
the prior bundle extraction.
Behaviour-preserving: ledger characterization snapshots and the repost item
valuation suite stay green.
Move serial & batch bundle handling (creation, validation, return bundles,
teardown) out of StockController into erpnext/stock/services/serial_batch_bundle.py
as a delegating service. The controller keeps thin delegators for the 10 methods
reached from other doctypes or run_method; the 12 internal-only helpers live in
the service. make_bundle_for_material_transfer stays a module fn in stock_controller
(imported by stock/serial_batch_bundle.py).
Behaviour-preserving: ledger characterization snapshots and the full Serial and
Batch Bundle test suite stay green.
Phase 0 safety net for the stock_controller service refactor. Captures the
combined GL + Stock Ledger output of representative stock vouchers (DN, Stock
Entry, Stock Reconciliation, Purchase Receipt incl. returns/taxes) as golden
snapshots, so later phases can prove ledger behaviour stays byte-identical while
stock_controller is split into services.
Run: bench --site <site> run-tests --module erpnext.stock.test_ledger_characterization
Regenerate goldens: REGEN_LEDGER_SNAPSHOTS=1 (after intentional changes only).
frappe-modifying-but-not-comitting anchors on the method definition, so the
suppression must sit on the def line (matching the convention used elsewhere
in the codebase); inner-line comments did not suppress it.
The golden-master snapshots, capture harness, characterization test, and
refactor spec existed to prove the accounts refactor preserved GL output.
All 29 characterization tests pass against the merged code, so the
scaffolding has served its purpose and is removed before merge.
Removes:
- erpnext/accounts/gl_snapshot.py
- erpnext/accounts/gl_snapshots/ (29 snapshots)
- erpnext/accounts/test_gl_characterization.py
- specs/accounts_refactor_spec.md
Both patterns are unchanged from develop but newly appear in the diff
because the refactoring relocated them:
- purchase_order/mapper.make_purchase_invoice_from_portal: portal flow
needs commit before redirect (matches develop behaviour)
- asset_repair.on_cancel: ignore_linked_doctypes is a runtime cancel flag,
not a persisted field
After moving mapping functions into per-doctype mapper.py modules and POS
logic into POSService, several call sites still referenced the old
locations, breaking import/collection in CI:
- bulk_transaction: import mapper modules for make_* transitions
- test_purchase_order / test_purchase_receipt / test_stock_entry: import
make_purchase_receipt, make_purchase_invoice, make_inter_company_purchase_receipt
and make_stock_entry from their mapper modules
- order.html: point portal API URL to purchase_order.mapper
- sales_invoice: add validate_full_payment delegating wrapper (called by POSInvoice)
It is Sales-Invoice-specific GL assembly and was the only TaxService
method called by the composer. Move it to SalesInvoiceGLComposer (verbatim),
call it as self.make_discount_gl_entries, drop the now-unused composer-level
TaxService local and the orphaned get_account_currency import in taxes.py.
Remove the doctype-branching make_precision_loss_gl_entry from
exchange_gain_loss.py (and its accounts_controller wrapper); add a
dedicated method to each of SalesInvoiceGLComposer and
PurchaseInvoiceGLComposer. The SI variant now passes 'Sales Invoice'
as the round-off voucher type (output-equivalent) and the throwaway
return value no longer shadows the gettext _ helper.
The make_/delete_/apply_loyalty_points methods on SalesInvoice only existed
as an inheritance surface for POSInvoice (self.X()). Route all callers
through LoyaltyService(doc).X() directly, consistent with how related-doc
cases already worked, and remove the three forwarding methods.
