A minimum order qty defined in stock UOM often has no exact
representation in the purchase UOM, so the smallest valid order slightly
exceeds the minimum. Surface that overage on the Purchase Order with a
toast on first save when an item's ordered stock qty is above its
minimum by less than one purchase-UOM step, so the buyer sees the
marginal increase before sending the order. Sub-precision dust stays
silent.
Its only caller, Purchase Order's get_items_from_open_material_requests,
was deleted in 91e9867fb1 (refactor: Cleanup buying module forms). The
old dotted path was already broken by the move to mapper.py, so no
external caller can be using it either.
The inverse (1 / value) and intermediate-UOM branches of
get_uom_conv_factor returned raw float quotients like
0.4535922921968971, bypassing the precision the docfields now declare.
Same for the client-side back-calculation from an edited stock qty.
Round both to the UOM Conversion Factor value precision.
The Float control parses values with the field precision, falling back
to the global float precision when the docfield declares none
(frappe ControlFloat.parse / get_precision). On a site with float
precision 2, a fetched UOM factor of 0.453592292 was written back to
the model as 0.45, silently corrupting every derived quantity by 0.8
percent. A ratio must not inherit display precision meant for
quantities, so declare the same precision 9 the UOM Conversion Factor
master already uses on every transaction-level conversion_factor
field.
cint truncates, so a stock_qty of 1999.9998 (dust from qty times
conversion factor) compared as abs(1999 - 2000.0) > epsilon and was
rejected as fractional even though it rounds to a whole number at
field precision, with the error confusingly printing the rounded
value: 'Quantity (2000.0) cannot be a fraction'. Round to field
precision first, then require the result to be a whole number.
Dust above an integer already passed; this fixes the asymmetry for
dust below.
stock_qty is stored as raw qty * conversion_factor, so a UOM-converted
order for exactly the minimum (e.g. LB to Kg) produces values like
1999.999999131832 vs a min_order_qty of 2000 and blocks the Purchase
Order. Round both sides to the stock_qty field precision before
comparing, and show the rounded qty in the error message.
Pick List, Material Request, Material Consumption and Additional
Material Transfer were spread across two standalone buttons and a
separate Make menu. Put them all under a single Create menu, and rename
Create Pick List to Pick List since the menu already says Create.
custom_make_buttons is updated to the new label so the connections
shortcut still finds the button.
Covers the _add_remaining_purchase_request path: partial stock in
another warehouse is allocated as a transfer and the residual purchase
qty goes through the second rounding site.
The stock-UOM qty is rounded in _accumulate_so_items, but the purchase
UOM conversion divided it by the conversion factor without re-rounding,
storing values like 5738748.300863984 in mr_items.quantity. The raw
value flowed into Material Request qty and the raw materials CSV, and
make_material_request compares quantity to requested_qty with exact
float equality, so any rounding downstream left dust quantities.
The division by conversion_factor in _adjust_required_qty_for_uom sits
directly after frappe.throw inside the same block, so it can never run.
It has been dead since commit 2a8cd05b44 (#27278) re-indented it into
the throw branch; the actual purchase-UOM conversion happens in
_material_request_item_row via _mr_purchase_conversion_factor.
* feat(accounts): split exchange gain and exchange loss accounts
Add optional Exchange Gain Account and Exchange Loss Account fields on
Company. When set, realized FX gain/loss from settling an invoice in a
foreign currency (via Payment Entry, Payment Reconciliation, or a
Journal-Entry-based advance) books to the matching account instead of
the single Exchange Gain/Loss account. Either field left blank falls
back to the existing Exchange Gain/Loss account, so companies that
don't configure the new fields are unaffected.
New companies get "Exchange Gain" and "Exchange Loss" accounts
auto-created in their chart of accounts and auto-assigned to the new
fields, same as the existing Exchange Gain/Loss account provisioning.
The Payment Reconciliation tool's per-allocation "Difference Account"
override in its reconcile dialog continues to work as before; the
split accounts only change the computed default shown there.
* test(account_balance): account for new Exchange Gain account in income report
The new auto-provisioned Exchange Gain account under Indirect Income
now shows up in the Income root type report for _Test Company 2.
---------
Co-authored-by: test <test@test.com>
* fix: use current batch avg rate for outward returns of batchwise valuation batches
* fix: honor zero batch average and avoid duplicate batch classification query
set_dynamic_labels() unconditionally forced update_stock's hidden
property based only on is_debit_note/has_subcontracted, overwriting
whatever Customize Form had set on every refresh. OR it with the
field's original (property-setter-driven) hidden value instead.
* feat: validate stock value and stock closing entry before period closing
* fix: do not accept scoped stock closing entries as period closing prerequisite
* feat: seed batch valuation from stock closing balance and freeze closed-period stock
get_mapped_doc copies every same-named field that is not no_copy, so the
Sales Order / Purchase Order / Quotation created from a Blanket Order
inherited MFG-BLR-.YYYY.- and was named MFG-BLR-2026-00003 instead of
SAL-ORD-2026-00001.
exclude naming_series from the mapping, same as job card does when it
maps to a Purchase Order.
fix: incorrect batch-wise valuation rate for entries with same posting datetime (#57794)
* fix: incorrect batch-wise valuation rate for entries with same posting datetime
The tie-breaker in get_batch_no_ledgers compared the bundle's creation
against the SLE's creation. These are different timelines - a bundle can
be created (drafted) much before its SLE (created at submission). For
entries sharing a posting datetime (backdated / amended vouchers), this
mis-ordered the entries against the ledger's replay order (SLE creation),
causing double counting or omission of batch qty / value and runaway
outgoing rates that no repost could heal.
Now the tie is broken using the creation of the bundle's own SLE (same
timeline on both sides). When the valuation runs through the bundle
before its SLE exists, the entry is by definition last in its timestamp
group, so all same-timestamp entries already in the ledger precede it.
* test: batch-wise valuation ordering for same posting datetime entries
Covers both tie-breaking branches of get_batch_no_ledgers:
- submission (pre-insertion) branch: same-timestamp inward at a different
rate plus a multi-row outward voucher (same item and warehouse), at
submission and after a backdated repost
- existing-SLE branch: a bundle created after its sibling's SLE, the
ordering must follow the SLE creation and not the bundle creation
Both tests fail with the previous parent.creation < sle.creation
tie-breaker and pass with the fix.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(subscription): don't reactivate a cancelled subscription
set_subscription_status() unconditionally set status to Active once
there was no outstanding invoice, even if the subscription had been
intentionally cancelled. Paying off an invoice issued before
cancellation (directly, or via the Payment Entry -> refresh hook)
flipped a Cancelled subscription back to Active while cancelation_date
stayed set.
process()'s cancel_at_period_end check compared posting_date against
getdate(self.end_date), and getdate(None) returns today, so an empty
end_date was silently treated as "cancel now" on every scheduler run.
Combined with the reactivation bug, this let a cancelled subscription
toggle Cancelled -> Active on each run and generate another invoice at
the next period boundary.
Fixes#57761
* fix(test): compare normalized dates in subscription cancellation test
cancelation_date read straight off an unsaved in-memory doc is a
string from nowdate(), but the same field comes back as a
datetime.date after reload(). Wrap both sides in getdate() so the
comparison isn't type-sensitive.
The toolbar handlers were copied onto view.events as unbound functions, so
`this` inside them was that object literal rather than the BOMConfigurator.
They worked only because the literal also carried `frm`, and broke as soon as
a handler called a method the literal did not list: get_item_code, added when
the tree started keying nodes on the row name, threw
"this.get_item_code is not a function" and killed Add Raw Material, Add Sub
Assembly and Convert to Sub Assembly.
Assign the instance instead of a hand-maintained whitelist. Every method is
reachable, `this.frm` keeps working, and no future method can be forgotten.
Fixes#57773
* fix: keep source rate on re-fetch when maintain same rate is enabled
With "maintain same rate" on, re-fetching item details on a row mapped from a
source document (e.g. a Purchase Order) pulled the latest Item Price, giving a
rate the document can never be saved with. Skip the price list fetch for such
rows and keep the source rate.
Fixesfrappe/erpnext#57436
* fix: keep source rate on bulk apply_price_list when maintain same rate is on
The single-row re-fetch guard skipped the bulk apply_price_list path, so
changing the price list, party, or conversion rate on a mapped transaction
re-fetched current Item Prices and overwrote the mapped rates, breaking the
maintain-same-rate check on save.
Guard apply_price_list_on_item with the same source-row lookup, and resolve the
parent doctype via ctx.parenttype since the bulk path carries the child doctype
in ctx.doctype.
* fix: preserve full source pricing on rate-locked rows
Restoring only price_list_rate on a mapped row dropped any manual discount or
margin, so re-running pricing produced a rate that differed from the source and
still failed the maintain-same-rate check on save.
Copy the source row's whole pricing block (rate, discount, margin) and skip
pricing rules for locked rows, in both get_item_details and the bulk
apply_price_list path.
* fix: pass child_docname in server bulk price apply so the rate lock is reachable
The server-side _apply_price_list builds its item ctx from as_dict(), which omits
the child_docname key the desk (JS) callers add, so the maintain-same-rate lock in
apply_price_list could not match rows in that path. Pass child_docname for
consistency with the desk callers.
* test: cover maintain-same-rate preservation on re-fetch of a discounted row
Reproduces the end-to-end symptom: a mapped Purchase Receipt row with a source
discount (rate != price_list_rate) keeps its rate after a re-fetch, so the
document saves under maintain-same-rate. Covers percentage and amount discounts
via process_item_selection, the server recompute the desk mirrors.
* fix: read the locked rate from the persisted source row
get_rate_locked_source_row returned the mutable target row, so an unsaved rate or
discount edit on a mapped row was preserved on re-fetch instead of the source
pricing, and the document still failed maintain-same-rate on save. Read the
pricing straight from the linked source row in the database, and cover the
edit-then-refresh case with a test.
* fix: permission-check the source row before returning its rate
The rate lock reads the linked source row with a direct db.get_value, which
bypasses permissions on a whitelisted endpoint. Only return the source pricing
when the caller can read the source document, so a crafted request cannot
disclose another document's rate. Covered by a test.
* fix: import make_purchase_receipt from its current mapper module
make_purchase_receipt moved from purchase_order.py to
purchase_order/mapper.py in a develop refactor pulled in by this
branch's merge commit. Two tests added afterwards still imported it
from the old path, failing CI with an ImportError.
* fix: Simplify source retrieval logic in get_item_details
Removed permission check for source parent in get_item_details.py.
* Revert "fix: Simplify source retrieval logic in get_item_details"
This reverts commit 58863805bd.
The test disabled the shared `Test Size` Item Attribute. On version-15
`FrappeTestCase` rolls back once per class instead of once per test, so the
flag stayed visible for the rest of `TestItem` and broke the seven tests that
build a variant from that attribute.
Build a dedicated attribute and template instead. Nothing the test writes is
reachable from another test, on either branch, so no cleanup is needed.
* fix(stock): allocate secondary item cost from the consumption entry
A secondary item's rate is its BOM share of the cost of the consumed
rows. With Get RM Cost From Consumption Entry enabled the consumption
happens in a separate document, so the Manufacture entry carries no
consumed rows and that cost is zero. The share evaluated to zero, and the
row fell through to the item's own valuation rate.
Only the finished good substituted the consumption entry's cost. Against
a consumption entry of 1000 and a BOM allocating 75% to the finished good
and 25% to scrap, the finished good took its 750 while the scrap took an
unrelated valuation of 100, booking 850 for 1000 consumed.
Derive the allocation base once and use it for both sides.
* test(stock): cover secondary allocation against a consumption entry
A consumption entry of 1000 splits into 750 and 250 by the BOM's shares.
* fix(stock): treat a 0% BOM cost allocation as no cost
A BOM splits its raw material cost between the finished good and its
secondary items, and validate_total_cost_allocation holds the two to
100%. An allocation of 0% therefore means the finished good takes
everything and the secondary item carries no cost.
The code read it as no allocation at all. A cost_allocation_per of 0 is
falsy, so the branch was skipped, the row kept a rate of zero, and the
fallback below handed it the item's own valuation rate. Producing 1000 of
raw material into a finished good at 100% and scrap at 0% booked 1000 to
the finished good and another 100 to the scrap.
Apply the BOM's share whatever it is, and mark the rate as derived so the
valuation fallback leaves a deliberate zero alone. rate_derived_from_consumption
becomes has_derived_rate, since it now guards more than the consumption case.
* test(stock): cover a secondary item allocated 0% of the cost
The finished good takes the full 1000 and the scrap row is worth nothing.
Assert that a variant saves after its attribute is disabled when the edit
leaves the attribute rows alone, and that changing an attribute value still
throws.
Disabling an Item Attribute writes `disabled = 1` into every Item Variant
Attribute row, including the rows on the template. `validate_variant` runs
on every save and walks the whole attribute table, so any later save of an
existing variant re-checked its untouched rows against the now-disabled
template row and threw. `update_variants` hit the same wall, which made a
single template save fail once an attribute was disabled.
The flag exists to keep an attribute out of new variants, not to freeze the
variants that already use it. item.js only reads it to drop the attribute
from the variant creation dialog.
Skip rows that are unchanged since the last save. New and edited rows are
still checked, so a disabled attribute cannot be added to an existing
variant, and the same guard covers the sibling checks for attributes and
values that the template no longer offers.
* fix(stock): stop treating a Repack secondary item as a finished good
mark_finished_and_secondary_items flagged every incoming Repack row as a
finished item, secondary rows included. Two things followed from that.
The row never reached the secondary-item branch in _set_incoming_item_rate,
so its own cost_allocation_per was never applied, and the BOM's
finished-good percentage was applied to every incoming row rather than
to the finished good alone.
Value was destroyed as a result. Repacking 1000 of raw material under a
BOM that allocates 75% to the finished good and 25% to scrap booked 500
to the finished good and 250 to the scrap: 750 in against 1000 out.
Leave secondary rows unflagged so each side takes the share the BOM
declares.
* test(stock): cover cost allocation for a Repack secondary item
A BOM allocating 75% to the finished good and 25% to scrap must split
1000 of raw material into 750 and 250, leaving no difference.
A Manufacture entry with a raw material worth 1000, a finished good and a
Scrap row typed in the UI must value the scrap at its own rate and take
that value out of the finished good, leaving no difference.
The legacy scrap checkbox deducted the scrap row's value from the
finished good, so a Manufacture entry balanced. Its replacement, the
Secondary Item Type dropdown, only balances when the row carries a BOM
Secondary Item link, because the cost allocation percentage lives there.
A row typed as Scrap in the UI has no such link, so its value was added
on top of a finished good that already absorbed the whole raw material
cost, and the entry closed with a non-zero difference.
Treat a secondary row with no BOM link the way the legacy scrap item was
treated: deduct its value from the finished good.
The finished good's rate is derived from the other incoming rows, so
those rows must be rated first. Previously the finished good was rated
in row order, ahead of the secondary rows, and picked up their amounts
only on a later validate pass. Rate the finished goods last so a single
pass is correct.
The inspection skip for secondary rows applied to every purpose, and in
validate_inspection it skipped the row even when the item itself mandated
inspection. Secondary Item Type is only meaningful on the purposes that
produce secondary items, but nothing clears it elsewhere, since
mark_finished_and_secondary_items runs for Manufacture and Repack alone.
A Material Receipt of an item marked Inspection Required Before Purchase
is blocked without an inspection. Setting Secondary Item Type on the row
submitted it clean.
Limit the exemption to the purposes that produce secondary items, and to
other doctypes such as Subcontracting Receipt, which carry the field with
its intended meaning. The client-side mirror is kept in sync.
Greptile flagged that the sales-side zero-qty-return fix had no dedicated
test proving the behavior - the existing suite happened to pass, but
nothing specifically asserted that an all-zero return is rejected while
a normal negative-qty return still succeeds.
Adds two tests covering the doctypes that rely entirely on this check
(no other guard covers them for a non-stock-effect return):
- Delivery Note return with qty 0 -> rejected
- Sales Invoice return with qty 0 (no update_stock) -> rejected
POS Invoice is not covered separately here since it always runs with
update_stock=1, which is already guarded by the pre-existing
validate_zero_qty_for_return_invoices_with_stock check regardless of
this fix.
validate_returned_items() set items_returned=True whenever a row matched
a valid item from the original document, even if its qty was 0. This let
a Sales Invoice, Delivery Note, or POS Invoice return be submitted with
every line at qty=0 - a no-op document with no stock or financial effect
that still consumed a document number and linked back to the original
transaction.
Scoped to the Sales side only: items_returned now flips to True for
Sales Invoice/Delivery Note/POS Invoice only when qty (or received_qty)
is actually negative, so an all-zero sales return correctly hits the
existing "At least one item should be entered with negative quantity"
check. Purchase Invoice, Purchase Receipt, and Subcontracting Receipt
are unchanged.
* fix(controllers): source trend report labels from the master
item_name, customer_name, territory and supplier_name are stored on each
transaction and editable, so they are not functionally dependent on the grouped
key and historical documents can hold different values for the same item,
customer or supplier. Aggregating them with Max() is a text sort, and MariaDB
folds case while PostgreSQL orders by byte value, so the two engines can label
the same row differently.
Read each from its master instead. Those values ARE dependent on the grouped
key, so they can be grouped without splitting rows and agree on both engines by
construction rather than by an assumption about the data. Supplier needed no new
join -- the Supplier master was already joined as t3 for supplier_group.
A Quotation's party_name is a dynamic link to either a Customer or a Lead, so
neither master can be joined without dropping the other; there the values come
from correlated subqueries over both, keyed only on the grouped party_name.
Row counts and every numeric total are unchanged. What changes is that a
renamed record now shows its current name rather than whichever historical
snapshot happened to sort highest.
* test(selling): assert which label the trends report returns
The existing tests assert the customer stays one row but never which territory
or name comes back, so a divergence between engines passes unnoticed. Asserts
both equal the Customer master's values while an order stores a different
territory.
* fix(controllers): resolve a Quotation's party label through quotation_to
party_name is a dynamic link, so looking it up in Customer and Lead alone was
wrong twice over: a Quotation raised against a Prospect or a CRM Deal got a
blank label, and when a Lead shared its name with a Customer the Customer-first
lookup returned the wrong record's name and territory.
Resolve through the quotation_to discriminator instead, mirroring
Quotation.set_customer_name -- Customer, Lead (company_name falling back to
lead_name), Prospect, and CRM Deal. The CRM Deal branch is emitted only when its
table exists, since it ships with the CRM app.
quotation_to joins the GROUP BY as well: two parties of different types can
share a name, and merging them into one row was never right.
* style(controllers): name the quotation CASE branches
semgrep's string-concat-in-list flags adjacent string literals inside a list,
since that shape is usually a missing comma rather than deliberate. Bind each
branch to a name first so the concatenation is unambiguous.
* fix(accounts): key the payment ledger CTEs on account, not Max(account)
QueryPaymentLedger builds two CTEs -- voucher amount and outstanding -- and
joins them on account among other columns. Both sides selected Max(account)
while grouping without it, so the join key was an aggregate over two different
row sets. A voucher posting ledger entries against two party accounts could
have the two sides pick different accounts, the join miss, and the outstanding
come back NULL. Max() over text is a sort, so which account wins is also
collation-dependent, and the engines sort text differently.
Group both CTEs by account instead. That makes the join key a real column and
scopes each Sum() to a single account -- so amount_in_account_currency is no
longer summed across accounts that may not share a currency. Row shape only
changes for a voucher that genuinely spans two party accounts for one party,
where today's single row is already an arbitrary pick over mixed currencies.
cost_center and remarks stay descriptive but genuinely vary per entry, and were
aggregated independently, so they could be stitched together from different
entries into a row that was never posted. They now come off one real entry,
picked by Min(name) -- Payment Ledger Entry declares no autoname rule, so
frappe names it by hash, and those are lower-case, which keeps the pick free of
the collation divergence.
* test(accounts): cover payment ledger metadata coherence
A Journal Entry posting two receivable lines for one customer with different
cost centers and remarks. Whatever row the ledger returns, its cost center and
remarks must be a pair that was actually posted. Guards the fixture itself, so
it cannot pass by posting only one distinct pair.
* test(accounts): cover the account-keyed payment ledger aggregation
The coherence test posts both party lines to one account, so it exercises the
representative-row metadata but not the account-keyed grouping or the CTE join.
Adds a Journal Entry posting to two receivable accounts for one customer and
asserts each account comes back as its own row, with its own amount and a
non-null outstanding.
Set the number format on the session user rather than on System Settings: the
code reads the user default, which shadows the global one, so these tests never
exercised the path they were written for. Restoring it in a finally also keeps
a failed assertion from leaving the whole suite in another locale.
Add a table test over every format in NUMBER_FORMAT_MAP, covering the grouped
values and the three formats parse_float used to read as 0, and restore the
formula-based coverage for non-numeric readings.
parse_float and is_valid_number each re-derived the number grammar, so the
validator accepted strings flt() cannot parse: str.isdigit() lets superscripts
through and lstrip("+-") lets repeated signs through, both then silently scored
as 0. One parse_reading() returning None when float() refuses the value makes
acceptance and conversion true by construction.
The grammar was also wrong for several formats. Where the group separator is
not a dot, a dot-decimal reading such as 1.15 parsed correctly before and is
accepted again. #,### and #.### report no decimal separator at all, which
rejected every fractional reading outright and, for #.###, reread a stored
1.500 as 1500.0; they now fall back to a dot and give up the grouping that
would collide with it.
Only readings that change are checked, so an inspection entered by a user in
one locale stays saveable and submittable by a user in another, and manual
inspection rows keep the free text they were never parsed for.
NumberFormat replaces get_number_format_info, which frappe drops in v16.
covers the reported case, a 1,15 reading in the space grouped "# ###,##"
format, which was read as 115 and rejected. also covers the dot grouped
comma format, and asserts that a reading written with the wrong separator,
or one that is not a number at all, is now rejected with an error rather
than read as a different value.
a numeric reading of "random text" was read as 0 and pulled the mean from
0.6 down to 0.4, which the test then asserted as accepted. such a reading
is now rejected outright, and the test is about formula evaluation, so drop
the row. its assertions are unchanged.
readings are Data fields, so they are parsed server side. parse_float only
swapped the separators for "#.###,##", so in the space grouped "# ###,##"
(polish) a reading of 1,15 was read as 115, fell outside the acceptance
range and silently rejected the inspection. strip whatever the group
separator is and normalise whatever the decimal separator is instead.
it also read the global number format, while the desk formats numbers with
the user's own. a user whose locale differs from the site therefore typed
readings in a format the server did not parse them with. read the user
default, which falls back to the global one.
a reading that is not a valid number in that format is now rejected with an
error instead of being read as a different number.
* fix(accounts): take POS summary warehouse and cost centre from one item line
Both describe an item line, not the invoice, and an invoice can carry several.
They were aggregated independently per invoice, so the report could show a
warehouse from one line beside a cost centre from another -- a pair that was
never posted.
The warehouse then becomes an outer grouping key, so the pick is not merely a
label: it decides how rows are partitioned across owner/date and therefore what
each row totals. Max() over text is a sort, and MariaDB folds case while
PostgreSQL orders by byte value, so the two engines can partition differently.
Take both off one real line instead, and the mode of payment off one real
payment line for the same reason. Sales Invoice Item is hash-named and Sales
Invoice Payment declares no autoname rule, so frappe hash-names it too -- which
keeps Min(name) free of the collation divergence that sorting text has.
* test(accounts): cover POS summary warehouse/cost-centre coherence
The existing tests post a single item line, so they cannot see this. Adds an
invoice with two lines whose warehouse and cost centre are deliberately
crossed: the higher warehouse sits on the line with the lower cost centre, so
an independently aggregated pair belongs to neither line.
* fix(accounts): pick the POS summary representative by idx, not by hash
Min(name) selected whichever child row happened to have the lowest hash, which
is arbitrary and turns on something unrelated to the data. Min(idx) selects the
first line the user actually entered: an integer, so the pick is free of
collation, and it is meaningful rather than incidental.
The join moves to (parent, idx), which is unique per parent.
The existing query tests assert only how many rows come back, so an ordering
divergence between engines passes unnoticed. Adds a case-adversarial pair: a
lead whose name starts with the search term in upper case, and one containing
it in lower case later on. The first must rank ahead of the second.
Reapplies #56330, which was reverted by #56389 with no recorded reason and has
been absent since 23 June.
The search filter uses .like(), which frappe renders as ILIKE on PostgreSQL, so
a candidate matches regardless of case. The ranking used a bare Locate(), which
frappe renders as strpos() -- case-sensitive there. A candidate can therefore
pass the filter, score no match in the ranking, fall back to 99999 and sort
last, while MariaDB's case-insensitive LOCATE ranks it first.
Same query, different order on the two engines, and a different result page
once page_len cuts between them.
Lower() both operands, matching the item, project, user and pick list handlers
in this same file, which were already correct.
Three helpers each managed their own dictionary on frappe.local, duplicating
cache lifecycle and key handling. @request_cache does the same thing centrally
and is cleared with the request, so the copies cannot drift apart.
Behaviour is unchanged: the decorator keys on the call arguments, which are the
same tuple each hand-rolled key was built from.
A BOM listing one item on two lines, with descriptions and source warehouses
that differ. The second line's description sorts above the first on either
engine, so an aggregated value would win; the row must instead carry the first
line's description together with that same line's warehouse.
Max() over a text column is a sort, and the engines sort text differently:
MariaDB's utf8mb4 collations fold case, the CI PostgreSQL orders by byte
value. MAX('abc','ABD') is 'ABD' on MariaDB and 'abc' on PostgreSQL --
confirmed on CI in the probe attached to #56241.
The parity effort wrapped many descriptive columns in Max() on the reasoning
that it returns the value MySQL picked arbitrarily. Where the column is
functionally dependent on the group key that holds and the wrap is a genuine
no-op. Where it genuinely varies -- description, item_name, uom and their
warehouses all describe a LINE, not the item -- it does not: MySQL picked a
row, not a maximum, and the sort now diverges between engines. Aggregating
each column separately can also pair one line's description with another's
warehouse, or a uom with the wrong conversion factor.
Take those columns from a single real line instead, the first by idx.
Only groups built from more than one line need it. Each query now also selects
Count(<line>.name).distinct(), and the representative pass returns immediately
when no group has more than one line -- in that case Max() of a single value
is already exact and collation cannot apply. A BOM with no repeated item
therefore issues no extra query at all, which matters because the explosion
and sub-assembly resolution recurse per sub-BOM. Genuine repeats are memoised
per request.
Sites covered: BOM explosion and sub-item queries, sub-assembly raw materials,
get_bom_items_as_dict, BOM Stock Analysis (both queries), Requested Items to
Order and Receive, Pending SO Items for Purchase Request, and Job Card
secondary items.
* fix(stock): take disassembly source columns from one posted line
get_items_from_manufacture_stock_entry collapses a work order's Manufacture
entries to one row per item and wrapped fifteen Stock Entry Detail columns in
independent Max() to satisfy Postgres' strict GROUP BY. Those columns describe
a line, not an item, and three sets have to stay together:
uom only means something beside its conversion_factor
batch_no and serial_no only beside their warehouse
is_finished_item decides whether the row is the output or an input
Aggregated separately they can be drawn from different lines. Two Manufacture
entries consuming the same item in Nos and in Box return ("Nos", 5) -- a pair
that was never posted, and one that does not describe the summed quantity.
Keep the sums (and the qty-weighted basic_rate) in the aggregate, and read the
descriptive columns off a single real line: the earliest by Stock Entry
creation then idx. That is what MariaDB returned in practice, it is
deterministic, and it is identical on both engines. Same representative-row
shape already used by BOM Stock Analysis and the sub-assembly queries.
* test(manufacturing): cover disassembly source-row coherence
Two Manufacture entries consume the same raw material in different UOMs, so
the max uom and the max conversion factor come from different lines. Asserts
the returned pair is one that was actually posted. Fails on the previous
per-column Max() with ('Nos', 5.0) not found in {('Nos', 1.0), ('Box', 5.0)}.
* fix(stock): aggregate disassembly quantities in stock UOM
* fix(manufacturing): stop BOM Stock Analysis inflating both its sums
get_bom_data left-joined Bin on item_code alone and then summed over the
result. Bin holds one row per warehouse and BOM Item one row per line, so the
join is a cross product and each SUM counts the other side's rows:
Sum(qty_consumed_per_unit) x (number of warehouses holding the item)
Sum(bin.actual_qty) x (number of BOM lines carrying the item)
A component on two BOM lines, stocked in two warehouses, reported a per-unit
requirement of 10 instead of 5 and available stock of 20 instead of 10 --
wrong on both engines, and wrong in the single-line case too as soon as the
item sits in more than one warehouse.
Aggregate Bin to one row per item_code before joining, so neither sum can see
the other's duplicates. The warehouse filter moves into that subquery; it
previously sat in the outer WHERE against a left-joined column, which
silently made the join inner, so the join is now made inner explicitly when a
warehouse is given to keep items with no bin there excluded as before.
* test(manufacturing): cover the BOM Stock Analysis bin-join cross product
Component on two BOM lines, stocked in two warehouses: the join yields four
rows, so both sums are doubled. Asserts qty_per_unit is the sum of the lines'
own per-unit quantities and actual_qty the real total across warehouses.
Fails on the previous single-query form with 10.0 != 5.0.
* fix(manufacturing): compute BOM item amount per line
get_bom_items_as_dict groups BOM lines by item_code, so a BOM listing the
same item on more than one line collapses to a single row. The amount column
multiplied the summed quantity by a single line's rate:
Sum(stock_qty / bom.quantity) * Max(rate) * qty
That is neither line's amount and not their total. The Max() was added to
satisfy Postgres' strict GROUP BY on the assumption that rate is constant per
item, but rate is editable per line.
Fold the rate into the sum so every line contributes its own:
Sum(stock_qty / bom.quantity * rate) * qty
Identical for the common single-line item, correct for duplicates, and valid
on both engines. Same class as the fix applied to budget_controller's
requested amount.
* test(manufacturing): cover BOM item amount across duplicate lines
A BOM listing the same item twice, once in the stock UOM and once in a UOM
with a conversion factor, gives the two lines different rates (rate is the
valuation rate scaled by the conversion factor). The two lines collapse into
one row in get_bom_items_as_dict, so amount must be the sum of each line's
own qty x rate.
Guards the fixture with an assertion that the two rates actually differ,
so the test cannot pass vacuously. Fails on the previous
Sum(stock_qty) * Max(rate) expression.
* fix(manufacturing): use matching UOM quantity for BOM amount
Max()/Min() over a text column is a sort, and the engines sort text
differently: MariaDB's utf8mb4 collations fold case, PostgreSQL as CI runs it
orders by byte value. MAX('abc','ABD') is 'ABD' on MariaDB and 'abc' on
PostgreSQL, confirmed on CI in the probe attached to #56241.
That makes a Max() over a text column which varies in case within its group a
live parity gap, rather than the arbitrary-pick preservation the wrap is
usually justified as. Where the column is functionally dependent on the group
key it stays a genuine no-op and collation cannot matter, so the rule is
scoped to non-FD columns to keep it a high-precision signal.
Recorded as a fifth second-order trap in the guide and in the Greptile
instructions, including the trap that a local macOS PostgreSQL agrees with
MariaDB here and reports a false all-clear.
GITHUB_REF is already the fully qualified ref for both branch and tag events,
so reconstructing refs/heads/$GITHUB_REF_NAME and refs/tags/$GITHUB_REF_NAME
just risks the two drifting apart. Keep the type check, since it still decides
whether the develop fallback applies, and take the ref verbatim.
The probe used --heads with a bare name, so it could not describe a tag push
and would have fallen back to develop for one. Resolve a fully qualified ref
from the event instead: the PR base or pushed branch under refs/heads, a tag
under refs/tags, and fail loudly on an unrecognised ref type.
Only branch refs are eligible for the develop fallback. A tag that is absent
from frappe is a real error, not a stacked-PR base, so it still fails.
The previous `||` treated every fetch failure as a missing branch, so a
transient network or auth error on a base that does exist in frappe would
silently substitute develop and report Patch Test results against the wrong
revision.
Probe with `ls-remote --exit-code` instead: exit 2 means no matching ref, so
fall back; any other non-zero status is a real failure and is re-raised.
The Patch Test fetches the frappe repo using this erpnext PR's base branch
name. For an ordinary PR that is develop, which exists in frappe/frappe. For a
stacked PR the base is an erpnext feature branch with no counterpart there, so
the fetch fails and the step exits 128 before any patch runs:
fatal: couldn't find remote ref pg-audit/bom-amount-per-line
This affects every stacked PR. It has been latent rather than absent: earlier
stacks passed only because their Patch Test ran while they still targeted
develop, before being retargeted onto the layer below.
Fall back to develop when the base ref does not resolve. Ordinary PRs and
version-branch PRs are unaffected -- their base exists in frappe, so the first
fetch succeeds and the fallback never runs.
* feat(job_card): carry the stock uom on the job card
Every quantity the job card reports belongs to the item it produces, but the
document had no unit of its own, so messages could only print bare numbers.
Add the Stock UOM field, set from the finished good or the final product, and
backfill the job cards that already exist.
* fix(job_card): print quantities with their unit
A bare 5 in an error says nothing about what was counted. Every message that
reports a quantity now names its unit, taking it from the job card's stock uom,
from the previous operation's finished good when the message compares two
operations, and from the item itself for a raw material transfer.
The completion dialogs read the same unit off the job card.
* refactor(job_card): move the stock uom next to the qty it measures
* fix(job_card): keep the stock uom backfill atomic
Drop the auto commit toggle so the backfill is a single transaction with no
connection flag left behind when it raises, and select the job cards to fill
with an explicit unset filter instead of a value list.
* refactor(job_card): drop the unused make_finished_good handler
Nothing triggered it and Job Card has no make_finished_good method to call.
* refactor(job_card): make the completion dialog say what it asks for
The dialog qty shares the Qty to Manufacture label with the field on the form
while it means the current cycle only, its title fell back to the generic Enter
Value because frappe.prompt takes four arguments and it was passed five, and
nothing on it stated that the three quantities have to add up.
Name the cycle in the label, title the dialog after the button that opens it,
and describe the split on the fields. Same wording in the shop floor dialog.
* fix(job_card): reject a completion split that cannot add up
The completion dialogs silently dropped a recalculation whose result went
negative, so entering a pending qty larger than what is left of the qty to
manufacture kept the contradiction (3 to manufacture, 3 completed, 2 pending)
and the job card only failed much later, on submission.
Keep the split consistent while it is entered: reset the pending qty when the
qty to manufacture changes, and refuse a completed, pending or process loss qty
that leaves the others negative. complete_job_card validates the same rule, so
the shop floor and the API cannot store a split that will never submit.
Also name the three parts in the submission error instead of calling their sum
the Total Completed Qty, which read as a contradiction of the field itself.
* test(job_card): cover the completion qty split guard
* fix(job_card): leave the pending qty out of the job card's own output
Pending qty is the part of a job card handed over to another job card, but the
status and the manufacturing entry still measured the card against its full
for_quantity. A card submitted with 3 completed and 2 pending was stuck at Work
In Progress with no way to change it, and its manufacturing entry was built for
the full 5.
Measure both against for_quantity minus pending qty, so the card reaches To
Manufacture on submission, its manufacturing entry covers the completed qty, and
it is Completed once that qty is manufactured.
* test(job_card): cover a job card completed with a pending qty
* fix(job_card): apply the completion dialog's qty to manufacture
Both the desk dialog and the shop floor session dialog send for_quantity when
completing a job card, but complete_job_card dropped it. Reducing Qty to
Manufacture to 3 on a job card of 5 left for_quantity at 5, so set_process_loss
turned the untouched 2 into process loss on the next save.
The dialog qty covers the current cycle, so add it to the qty already completed
by the earlier cycles of the job card instead of overwriting for_quantity, and
validate the pending qty against the result.
* test(job_card): cover qty to manufacture from the completion dialog
Reducing the dialog qty resizes the job card without inventing process loss, and
a pending qty split across two cycles leaves for_quantity untouched.
* fix(job_card): block next operation until previous operation is manufactured
With track semi finished goods, Work Order Operation completed_qty is set from
the submitted job cards' total completed qty, so a job card of the next
operation could be started and completed even when no Manufacture entry existed
for the previous operation. The semi-finished goods it consumes were never
produced.
Validate the sequence against the qty actually manufactured against the previous
operations' job cards (Manufacture entries / Subcontracting Receipts) when the
work order tracks semi finished goods.
* test(job_card): cover manufactured qty check across previous operations
Work order with operations A and B at sequence 1 and C at sequence 2, tracking
semi finished goods. C stays blocked while A's job card is submitted but its
Manufacture entry is missing, and once A is manufactured for 3, C can only be
completed for 3.
* ci: fall back to develop when frappe has no matching branch
The frappe branch to install is taken from the pull request's base branch. A
stacked pull request targets another erpnext branch, so the clone fails with
"couldn't find remote ref", no bench is installed, and every job that needs one
fails with it.
Fall back to develop when the base branch does not exist in frappe. An explicit
FRAPPE_BRANCH is left alone, since it can be a commit sha rather than a branch.
* ci: only fall back when frappe is known to lack the branch
git ls-remote --exit-code reports 2 for a branch that is not there and 128 for a
remote it could not reach. Treating both as absence let a transient network or
DNS failure install develop over the branch the pull request was built against.
Fall back on 2 alone and log anything else, so a flaky probe leaves the branch
as it was.
A Material Request where few items carry a default supplier meant picking the
same supplier row by row. A Supplier field above the table copies its value
into every row, leaving the exceptions to be corrected by hand.
Both pickers skip suppliers that are disabled or barred from Purchase Orders by
their scorecard standing.
Creating through the dialog calls the endpoint directly instead of going
through open_mapped_doc, so the draft link guard that every other Create action
runs never fired, and a repeated dialog quietly produced a second set of draft
orders for the same quantity.
Each row was checked against the pending quantity on its own, so a payload that
listed one item under two suppliers passed both checks and ordered the pending
quantity twice. The dialog cannot produce that, a direct call to the endpoint
can.
Naming a single order in a message and leaving the buyer to click it is a step
for nothing. The form opens directly when there is one order; the message stays
for the case it was meant for, several orders at once.
Every row is ticked when the dialog opens, so the common case of ordering
everything is unchanged, and a buyer who wants a partial order unticks what
should wait. Creating with nothing ticked is rejected.
A bare item code left the buyer to find the item themselves, and a bare number
gave no clue what the limit was counted in. Both messages now link the item and
state the pending quantity in bold with its UOM.
Opening one of several created orders hid the rest and moved the buyer off the
Material Request. The created orders are now reported the way Production Plan
reports its documents, as links in a message, and the form stays put.
Mapping drops a schedule date that already passed, leaving the buyer to pick a
new one on the Purchase Order form. Nothing fills it in when the orders are
created straight from the supplier selection dialog, so a Material Request
whose required date has gone by failed to save with "Please enter the Required
By".
Items that lose their date now fall back to today, which is the earliest date a
Purchase Order raised today accepts.
Asserts the requested quantity reaches the Purchase Order item and that rows
without a supplier, or with a quantity that is zero, negative or beyond the
pending quantity, are rejected.
The dialog prefilled the pending quantity of each Material Request item but
kept it read only, so ordering less than what was requested meant editing the
Purchase Order afterwards.
The quantity is now editable and is validated against the pending quantity of
its Material Request item, both in the dialog and on the server. The requested
quantity is handed to the mapper as the pending quantity of the source row, so
the existing mapping - including the subcontracting conversions - derives the
Purchase Order quantities from it unchanged.
Covers the default supplier lookup for pending items, the supplier passed
through to a single mapped order, the grouping of items into one order per
supplier, and the failure when an item is sent without a supplier.
Creating a Purchase Order from a Material Request mapped every pending item
into a single order, leaving the buyer to split it by hand whenever the items
came from different vendors.
The Create action now reads the default supplier of each pending item (item,
item group, then brand defaults). When the items resolve to more than one
distinct supplier - including the case where only some of them have a default -
a dialog lists the items with their default supplier prefilled and editable.
Submitting it groups the items by the chosen supplier and creates one draft
Purchase Order per group.
When every item resolves to the same supplier the order is mapped straight
away with that supplier set, and when none of them has a default supplier the
previous behaviour is unchanged.
calculate_item_values rounds every Float field on an item row to the
site's Float Precision (3 by default), and conversion_factor was one of
them. The factor is a ratio, not a rate: UOM Conversion Factor.value is
stored at precision 9, and Material Request keeps the full value because
it has no currency field and so never runs the calculation.
Mapping a Material Request to a Purchase Order therefore truncated the
factor - 0.453592292 for Pound -> Kg became 0.454 - and stock_qty, which
is recomputed as qty * conversion_factor, drifted from the quantity that
was requested, leaving the Material Request unable to close.
Exclude conversion_factor from the rounded fields on the server and on
the client. Factors below the site precision would otherwise round to
zero outright.
validate_returned_items() set items_returned=True whenever a row matched
a valid item from the original document, even if its qty was 0. This let
a Purchase Invoice, Purchase Receipt, or Subcontracting Receipt return be
submitted with every line at qty=0 - a no-op document with no stock or
financial effect that still consumed a document number and linked back
to the original transaction.
Scoped to the Purchase side only: items_returned now flips to True for
Purchase Invoice/Purchase Receipt/Subcontracting Receipt only when qty
(or received_qty) is actually negative, so an all-zero purchase return
correctly hits the existing "At least one item should be entered with
negative quantity" check. Sales Invoice, Delivery Note, and POS Invoice
are unchanged.
Also applies a corresponding check to the item_name-only fallback branch
(for rows without an item_code - Item Code is not mandatory on Purchase
Invoice Item), which previously bypassed this fix entirely and still set
items_returned=True unconditionally regardless of quantity. For that
branch specifically, only qty is checked (not received_qty): with no
linked Item there's no accepted/rejected split, so received_qty carries
no independent meaning and a qty=0 row must be rejected regardless of
its value.
Filter Accounts Receivable and AR Summary on the Sales Invoice's own
sales_partner instead of the customer's default_sales_partner, and read
the Sales Partner column from the invoice. Returns are attributed to the
invoice they settle, matching how the Sales Person filter works.
check the result of find() before reading t_warehouse off it. on a
'receive from customer' entry with no row carrying scio_detail, find()
returns undefined and items_add throws a typeerror.
the throw rejects the serially-run handler chain, so the stock entry
controller's own items_add never runs and the new row silently loses
its target warehouse, expense account, cost center and serial/batch
field defaults.
leave t_warehouse unset when no reference row exists, so the rest of
the chain still runs.
When a plan is selected in the Subscription's Plans table, the Subscription's
accounting dimensions (cost center and any custom dimensions) auto-fill from the
plan, falling back to the plan item's company default (selling cost center for a
Customer, buying for a Supplier). Only empty fields are filled. Stale async
responses are ignored so a quick re-pick of the plan can't be overwritten.
install_fixtures always inserted "All Item Groups" as a parentless group.
On a site where another app had already created the root, ItemGroup.validate
re-parented it, leaving a second group-root that held the standard groups
while the real root held everything else.
This is reproducible with the healthcare app on a non-English site: its
after_install seeds the root as _("All Item Groups"), so a pt-BR site gets
"Todos os Grupos de Itens" as the root before the setup wizard runs. The
split predates #57390 -- the old translated-name lookup resolved to the same
root and produced an identical tree.
Resolve the root once with get_root_of (falling back to the canonical English
name on fresh installs) and use it for the root record's exists-guard and the
standard groups' parent, matching Company.create_default_departments.
Patch merges an already-seeded "All Item Groups" into the root it sits under,
lifting its children and repointing every link.
Closes#57581
`set_title()` runs in validate and always fills `title` first, so
`set_title_field()` never renders `{material_request_type}`, and
create_new.js skips defaults for the field `title_field` names. Titles
stay "<type> Request for <items>".
Timesheet was the only doctype where the {...} template on `title`
actually rendered: `title_field` was `title`, so `set_title_field()`
seeded it from `{employee_name}` on insert. A `default` renders once, so
reassigning a draft left the stored title — and every label derived from
it — on the previous employee, with no way to correct it from the form
because the field is hidden.
Point `title_field` at `employee_name` so the label reads the live field
instead of a copy that drifts. Existing rows need no backfill.
Whether a return row could be closed was decided by the sign of its amount
rather than by anyone's intent: the signed comparison happened to reject it,
and switching to magnitudes happened to allow it. Closing a whole return
document was already allowed, so allowing the row is the consistent choice,
but it should be written down.
`is_item_closable` now says so, and the behaviour is covered on both Delivery
Note and Purchase Receipt returns rather than only the one that was reported.
Also asserts the property worth protecting: closing a row on a return leaves
the original document's returned qty and per_returned untouched.
The server started treating billing amounts as magnitudes so return rows stay
closable, but the client kept the signed comparison. An unbilled return row was
filtered out of the Close Items dialog and hid the button entirely, so the row
the server would accept could not be reached from the form.
The pending amount shown in the dialog had the same flaw and would have
displayed zero outstanding on a row with a full amount left to credit.
Verified on a return Delivery Note: the row now appears with amount -500 and
pending amount 500.
Two issues from review.
The endpoint stored whatever integer the caller passed, so `closed=2` was
treated as closed by every truthy check while `validate_closed_source_items`
looks for exactly 1. A submit-authorized caller could suppress a row from
mapping and still submit an invoice against it. Verified: the value persisted
as 2, truthy checks saw it as closed, the guard query did not find it.
`is_item_closable` compared signed amounts, so an unbilled return row with a
negative amount looked already completed and could not be closed. Verified on a
real return Delivery Note: amount -500, billed_amt 0, predicate false.
Magnitudes are compared instead, matching how the percentage funnel already
treats these fields.
Removes the duplication left over from adding the doctypes one at a time.
`is_bundle_of_closed_row` was copied between the Sales Order and Delivery Note
mappers, differing only in a doctype name; it now derives that from the packed
item's parenttype. `is_item_closable` was identical on Delivery Note and
Purchase Receipt and repeated the billing clause on the order doctypes; billing
is now the default on AccountsController and the orders add their own
fulfilment axis. The close dialog config was written out per doctype, differing
by a qty field, a column label and a sentence, and is now two builders in
erpnext/public/js/utils/item_close.js.
Also stops counting closed rows as committed spend in the budget's ordered
amount, which sums per row but only guarded the parent status.
When every row is closed there is nothing left to measure against, so the
percentage falls back to the whole table and reports what actually happened.
Writing off two unbilled rows leaves per_billed at 0; writing off two rows that
were fully received leaves per_received at 100 and per_billed at 0. A constant
would have been wrong in one direction or the other.
A closed row was counted as fully settled, so closing one of two unbilled
Delivery Note rows pushed per_billed to 50 and the document read "Partially
Billed" with nothing invoiced. The same inflation applied to per_received,
per_delivered and per_picked.
Closed rows now leave the denominator instead of counting as done, so a
percentage stays a true measure of what was received, delivered or billed
against what is still expected. Writing off one of two unbilled rows leaves
per_billed at 0; billing the other takes it to 100. When every row is closed
nothing is outstanding, so the percentage is 100 by definition.
This mirrors how Delivery Note already excludes returned value from the
billing denominator.
Amending a cancelled document no longer inherits its write-offs. Frappe copies
no_copy fields when amending so a document can be corrected and resubmitted,
and unlike billed_amt or received_qty nothing recomputes `closed`, so the flag
would silently keep a row out of billing on the new document.
Extends row level close to the two documents where the goods have already
moved, so closing a row writes off what is left to bill rather than what is
left to fulfil. Nothing is released in Bin.
Delivery Note and Purchase Receipt are billed through their own services
rather than through status_updater, so their invoices declare the row link in
`closed_source_links`. Without it a closed row stayed invoiceable, since the
existing guard only walked status_updater args.
`per_returned` shares the percentage funnel on both doctypes and is excluded
from `SETTLED_BY_CLOSE`: closing a row writes off pending billing, it does not
turn the row into a return.
Closed rows now show a grey indicator in the items grid on all four doctypes.
Purchase Receipt had no indicator formatter at all and gets one.
Extends row level close to Sales Order. Closing a row releases its reserved
qty from Bin, settles its delivery and billing progress, and skips it when
creating a Delivery Note, Sales Invoice, Pick List, Material Request or
drop ship Purchase Order.
A row holding Stock Reservation Entries cannot be closed. Releasing physical
stock is a deliberate act, so the row has to be unreserved first rather than
having its reservations cancelled as a side effect.
Packed Items have no flag of their own and follow the Sales Order Item row
that bundles them, both in the mappers and in the reserved qty rollup.
Closed rows count as picked in the picking percentage, so the Pick List
button stops offering a list that would map no rows.
The close dialog moves to erpnext/public/js/utils/item_close.js and is shared
with Purchase Order, with each doctype supplying its own eligibility rule,
help text and columns.
REOPEN_STATUS now records why the per doctype value matters: Sales Order
re-checks the credit limit only on the literal "Draft", so each doctype reuses
whatever its own Re-open button passes.
on_cancel pre-blocked cancellation with its own "Purchase Invoice is
already submitted" guard, duplicating the check Frappe already runs for any
submitted linked document. Drop the guard and the unused check_next_docstatus()
method it mirrored so the receipt defers to the framework: the Cancel All
Documents flow cancels the invoice first and then the receipt, and a direct
cancel is still rejected by Frappe's linked-document check.
Add a regression test that a direct cancel of a receipt with a submitted
invoice is rejected and rolls back, leaving no stray stock or GL entries.
Adds a `closed` flag on Purchase Order Item so a single line can be written
off without closing the whole order. Closing a row settles it: its pending
quantity stops holding the order open, its ordered qty is released from Bin,
and it is skipped when creating a Purchase Receipt or Purchase Invoice.
The percentage funnel in StatusUpdater counts a closed row as fully settled,
gated on the progress field so `per_returned` is unaffected — closing writes
off what is pending, it does not turn a row into a return.
Parent and row close stay independent owners: a closed parent does not stamp
its rows, and consumers check both. Closing the last open row closes the
parent; reopening any row reopens it. Reopening a parent whose rows are all
closed is blocked, since it would read as open while every row stayed
suppressed.
Rows that are already received and billed in full cannot be closed, matching
the existing document level gate. Rows received but not yet billed can be,
which writes off the remaining billable amount.
* test: isolate accounts settings mutation in overdue threshold test
test_overdue_billing_threshold_on_submit mutated the Accounts Settings singleton
without restoring it, so a failed assertion mid-test leaked
enable_overdue_billing_threshold and the bypass role into later tests that submit
sales invoices. Wrap the mutations in try/finally and restore the originals. Also
assert that a 0 overdue limit on the customer inherits the customer group's limit.
* test: restore credit limits in overdue threshold fallback test
validate_sample_quantity and move_sample_to_retention_warehouse are
whitelisted and take company from the caller, which selects whose retention
warehouse gets read. The retained batch qty then reaches the return value and
the max-retained warning, so an authenticated user could probe another
company's stock with a known item and batch.
Gate the shared company -> warehouse resolution on read permission for the
Company, which respects User Permissions. validate_sample_quantity only grew
a company argument in this branch; move_sample_to_retention_warehouse already
took one, so this closes that path too.
Companies now get their Stores warehouse as Default Warehouse, so item
warehouse resolution succeeds where it previously came back empty.
test_internal_pr_reference cleared inter_company_reference and asserted a
ValidationError, but no validation covers that field - the mapper already
sets it. It was incidentally catching 'Row #1: Warehouse is mandatory for
stock Item' from the blank target warehouse, so the assertion never tested
what it claimed. Dropped it; the delivery_note_item assertion below still
covers the reference linkage.
test_inter_company_transaction_without_default_warehouse now establishes its
own premise by clearing the company's default warehouse instead of relying
on it being unset. Its failure previously skipped the teardown that restores
frappe.local.enable_perpetual_inventory, which db rollback cannot undo, which
in turn broke two later inter-company tests.
Item.retain_sample is validated against any company having one configured,
since Item is not company-scoped. The transaction company may still not be,
in which case get_batch_qty received warehouse=None and returned its batch
list, which was then compared numerically -> TypeError.
Resolve company -> retention warehouse through one helper that throws a
clear message instead, covering both the sample stock entry and the
whitelisted quantity validation.
Default Warehouse and Sample Retention Warehouse were global singles, so every
consumer had to re-check that the warehouse belonged to the transaction's
company before using it. Both now live on Company, under a new Warehouse
Defaults section that also collects the warehouse fields Company already had.
The company check moves to Company.validate_warehouses, which also rejects
group warehouses for all seven fields — a group or cross-company value there
already failed at SLE time, this just surfaces it at the source.
New companies get their Stores warehouse as Default Warehouse via
create_default_warehouses, replacing the setup-wizard and test-fixture code
that seeded the global.
row defaults to None but was dereferenced unconditionally when computing
the incoming rate for batch-tracked items, causing an AttributeError for
any caller (e.g. direct API calls) that omits row while still passing
batch_no. The other two row accesses in this function already guard
against None; this brings the incoming-rate check in line with them.
Production Plan read the conversion factor straight off the item's own
UOM child table, so an item with a purchase UOM but no matching row threw
"UOM Conversion factor not found" while Stock Entry silently resolved it
from the item's variant template or the UOM Conversion Factor doctype.
Resolve it the same way, and keep returning None when nothing is
configured anywhere so the missing-setup error still fires.
Default WIP, Finished Goods and Scrap Warehouse fields on Company listed
warehouses of every company. Filter them by the current company and
exclude group warehouses, matching the other warehouse fields.
`get_linked_dunnings_as_per_state` joins Dunning to its Overdue Payment child
table without DISTINCT. When a Sales Invoice has more than one overdue
installment, its Dunning holds one Overdue Payment row per installment, so the
query returns the same Dunning name once per row.
`update_linked_dunnings` then loads that Dunning name into a separate document
object for each duplicate row and saves each one. The first save bumps the
`modified` timestamp, so the second (now stale) save fails with
`TimestampMismatchError` ("Document has been modified after you have opened
it"). The error is raised on the Dunning while the user is submitting a Payment
Entry, making it confusing, and payments for such invoices cannot be posted at
all.
Add DISTINCT so each linked Dunning is returned (and saved) exactly once.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Pricing Rule: log a broken condition instead of dropping the rule
- Payment Request: log gateway validation failures
- Supplier Quotation from RFQ portal: let errors surface instead of
returning None to a dead submit button
- Bank Transaction Rule: drop the pointless try/except around the
on_trash unlink; a failed write already aborts the delete
- Statement of Accounts: the CC handler was unreachable
(frappe.get_value returns None, it does not raise); drop it and
filter out users without an email
- Bank Statement Import: 'Bank Account' at column 0 is falsy, so the
column was appended instead of filled
`calculate_rm_cost` skipped rate refresh whenever `bom_creator` was set,
so neither the Update Cost button nor the BOM Update Tool could ever
refresh those BOMs. Every BOM in a multi-level tree carries the field, so
whole trees stayed frozen at their creation rates.
The guard replaced the removed `rm_cost_as_per == "Manual"` check in
0b63dbf, on the assumption that BOM Creator rows hold manual rates. They
do not: BOM Creator recomputes every row from `rm_cost_as_per` on save.
The BOM Creator tree identified a node by the parent's item code
(fg_item) instead of the specific BOM Creator Item row, so every
occurrence of a repeated sub-assembly shared one child set: expanding
any one of them listed the raw materials of all of them, and deleting
one wiped the raw materials of its siblings.
Key the tree on fg_reference_id and make the node value the row name,
matching the framework convention that a tree node's value is its
docname. Item code now travels as its own field for the label and for
the fg_item argument sent back on add/convert.
Fixes#57311
update_semi_finished_good_details assigned the current job card's
manufactured_qty to Work Order.produced_qty instead of accumulating it,
so a second job card on the same operation overwrote the first. Nothing
corrected it afterwards because StatusService.update_work_order_qty
returns early for track_semi_finished_goods work orders, leaving the
work order stuck below its planned qty with no way to progress.
Aggregate manufactured_qty and completed_qty over the operation's
submitted job cards instead.
Subcontracting Receipt was left out of #57493. Its title_field is "title",
so the template does get rendered on insert, but never again — the title
goes stale as soon as the supplier changes.
Point title_field at supplier_name like Purchase Receipt, and give the
title field the same shape as its subcontracting siblings. Existing rows
already hold a rendered name, so no data patch is needed.
Also guard the whole class of bug: a "{...}" default on a title field is
only ever rendered when title_field is "title".
Point set_default_print_formats() at the new builder-made
"<Doctype> Modern with Images" formats, falling back to the existing
"with Item Image" formats when the new ones aren't present. Only fresh
sites are affected (the existing default-print-format guard is kept).
* fix(stock): keep manufactured item rate at zero when inputs are free
when a finished item is produced from raw materials consumed at zero
valuation, the incoming rate fell back to the item's own valuation
rate (or BOM cost), valuing free inputs as output and inflating the fg
value on every production run.
add has_consumption_basis() to detect when the consumed cost is known
even if it is zero (consumed rows present, or a consumption entry
exists for the work order). when it is, skip the get_valuation_rate and
BOM-cost fallbacks so a real cost of zero is preserved.
* test(stock): cover manufacture rate for zero-valued raw materials
- manufacture from a free input keeps fg basic_rate and sle
incoming_rate/stock_value_difference at zero even when the fg already
carries a valuation in the target warehouse
- material consumption on with no consumption entry does not fall back
to bom/price-list rate for free inputs
- zero-valued consumption entry keeps the manufacture entry's fg rate
at zero
* fix(stock): value batched packed-item returns from the original bundle
when a return delivery note or sales invoice bundle is built via the
use_serial_batch_fields / sle-driven path, its voucher_detail_no keeps the
packed item instead of being remapped to the parent dn/si item. the return
valuation lookup then misses and the bundle values at zero, so the sle
stock_value_difference stays wrong even after a repost.
resolve the original dn/si item via the packed item's parent_detail_docname
when the direct lookup fails, so the return values from the original outward
bundle on both submit and repost.
* test(stock): cover batched packed-item return valuation on repost
projected_qty is derived from every bin quantity, so refreshing only
reserved_qty_for_production_plan leaves it stale wherever another field
had drifted. Call Bin.recalculate_values() instead.
Renamed so the patch re-runs on sites that already applied
recompute_production_plan_reserved_qty.
A batch is one valuation pool, so any per-slot value difference within a
batch is stale detail from the report's own age slots, not real valuation.
The rebalance only ran when consumption had already driven a slot negative,
so a batch whose receipts landed at different rates kept a skewed split
across age buckets (one bucket free, another double-priced) while the total
stayed correct.
Drop the negative-slot precondition and always spread a batch's pooled value
over its slots in proportion to qty. Redistribution preserves group totals,
so buckets still sum to Stock Balance; only the split across ages changes.
subcontracting order and subcontracting inward order carry a hidden
title field defaulting to "{supplier_name}" / "{customer_name}", while
their title_field points at supplier_name / customer_name. document.
set_title_field() substitutes the template only when title_field is
"title", so every record stores the placeholder verbatim.
drop the dead default and hidden flags, move title into the other info
tab to match purchase order, and add a patch to repair existing rows.
the bin reserved-qty recalc filtered out closed purchase orders but not
closed subcontracting orders, so closing a partially-received sco kept the
reservation for the unreceived qty and left projected qty understated.
apply the same closed-status filter to the subcontracting order path.
fix: enable the 'Include Zero Stock Items' filter by default to show zero-stock items in the Stock Balance report (#57458)
(cherry picked from commit 4e8f5de5cb)
Co-authored-by: rohitwaghchaure <rohitw1991@gmail.com>
The whitelisted endpoint declared trans_items as str, so Frappe's typing
validation raised FrappeTypeError when the client sent the items as a
JSON list. ChildItemUpdater.update already handles both via
frappe.parse_json, so widen the wrapper's hint to str | list.
The right-column Custom HTML block re-displayed the vendor name a
second time. In the source Purchase Order design that block was part
of an address card (name + mailing address together); Request for
Quotation has no per-supplier address field, so after dropping the
address the block became a bare, purposeless repeat of the name
already shown at the top of the left column. Classic, Modern, and
Modern with Images each show it once - Bordered now matches.
Addresses the Greptile review comment on this PR.
Request for Quotation has no pricing fields at all (no rate, amount,
grand_total, in_words), so this is a structurally simplified version
of the Bordered/Classic/Modern/Modern with Images designs rather than
a straight field-swap: Sub Total/Discount/Tax/Grand Total/In Words
sections dropped entirely, item table shows Item/Code/Quantity only.
Supplier fields point at the doctype's own `vendor` field (matching
the existing request_for_quotation_with_item_image standard format's
convention), items table bound to Request for Quotation Item, status
field carries the real RFQ status list, no letter head embedded.
Adapted from the Sales Order formats: customer_name relabeled to
"Party Name" (Quotation supports both Customer and Lead via
quotation_to), delivery_date mapped to valid_till, items table bound
to Quotation Item, status field carries the real Quotation status
list, no letter head embedded.
Both are Data on the Purchase Invoice doctype, not Small Text
(carried over from the Sales Invoice formats these were adapted from,
where Small Text is correct - Sales Invoice's own doctype defines it
that way). No rendering impact: Data.html and Field.vue never branch
on Small Text vs Data. Addresses the Greptile review comment on
purchase_invoice_bordered.json, and applies the same fix to the other
three formats that had the identical issue.
Both are Data on the Purchase Order doctype; carried over as Small
Text from the Sales Order formats these were adapted from. No
rendering impact - caught via a proactive fieldtype audit after a
Greptile comment on a downstream PR flagged the same pattern on
Purchase Invoice.
Both are Data on the Delivery Note doctype; carried over as Small Text
from the Sales Order formats these were adapted from. No rendering
impact - caught via a proactive fieldtype audit after a Greptile
comment on a downstream PR flagged the same pattern on Purchase
Invoice.
Both are Data on the Sales Order doctype; they were carried over as
Small Text from the Sales Invoice formats they were adapted from,
where that type is correct (Sales Invoice defines them as Small Text).
No rendering impact — caught via a proactive fieldtype audit after a
Greptile comment on a downstream PR flagged the same pattern on
Purchase Invoice.
Adapted from the Sales Invoice formats: nearly identical field shape
(customer_name, address_display, company_address_display, posting_date,
due_date, total, in_words all exist natively on POS Invoice), so only
the items table (bound to POS Invoice Item), status field options, and
title text needed changing. Each format correctly relabels as "Credit
Note" instead of "POS Invoice" when is_return is set. Field types for
customer_name/in_words corrected to Data to match POS Invoice's own
doctype definition (Sales Invoice defines these as Small Text; POS
Invoice does not). No letter head embedded.
Adapted from the Sales Invoice formats for the buying side: customer
fields swapped for supplier fields, items table bound to Purchase
Invoice Item, status field carries the real Purchase Invoice status
list. Each format correctly relabels as "Debit Note" instead of
"Purchase Invoice" when is_return is set, mirroring the Credit Note
labeling on the Sales Invoice formats. No letter head embedded.
Adapted from the Sales Order formats for the buying side: customer
fields swapped for supplier fields, delivery_date mapped to
schedule_date (Required By), items table bound to Purchase Order
Item, status field carries the real Purchase Order status list, no
letter head embedded.
Adapted from the Sales Order formats: transaction_date/delivery_date
mapped to Delivery Note's own posting_date and po_no (customer's PO
number), items table bound to Delivery Note Item, status field carries
the real Delivery Note status list, no letter head embedded.
- items table options: Sales Invoice Item -> Sales Order Item (all four formats)
- status field options: replace the Sales Invoice status list with the actual
Sales Order statuses (Modern, Modern with Images)
- drop the redundant document-number field from the Modern footer; the header
already shows it
Adapted from the new Sales Invoice Bordered/Classic/Modern/Modern with
Images formats: invoice-specific fields (posting_date, due_date, the
Sales Invoice/Credit Note title) mapped to their Sales Order
equivalents, no letter head embedded.
use frappe.get_list instead of frappe.get_all in get_dashboard_info so
the company list honors user permissions. previously, a party with
invoices across multiple companies would raise "User don't have
permissions to select/read this account" for users restricted to a
subset of companies, since get_party_account was called for companies
the user could not access.
fixesfrappe/erpnext#57428
- drop dev-site-specific letter_head references so a fresh install picks its own default
- add missing row_condition on Modern with Images' tax repeater
- standardize Sub Total on total (matches summed item amounts) and In Words on the transaction-currency field across all four formats
Address review findings:
- send_proforma_email rejects non-issued proformas, and the tab suppresses the
action for cancelled rows, so a voided document can't be sent to a customer
- mark proforma_pdf as no_copy and disable amendment (a proforma is created
only from a Sales Order), so a copied proforma can't carry the original's PDF
and number
Address review findings:
- make_proforma_invoice: reject a non-submitted Sales Order (the whitelisted
endpoint was previously only JS-gated on docstatus)
- send_proforma_email: throw a clear error when the attached PDF File is missing
instead of passing a null fid to sendmail
sort on (expiry is none, expiry) so a null expiry_date is never
order-compared against a datetime.date, which raised typeerror in
python 3 when a warehouse held both dated and never-expiring batches.
warehouse_name is a Data field shown as stored, and everywhere else
users see the docname, which carries the company abbr ("Stores - F")
and is never translated - a translated bare "Stores" has no display
use, so the identity _ marking from #57392 serves nothing. The
exists-guard keeps the runtime session-translation match that protects
legacy sites from duplicate English warehouses.
A batch is one valuation pool, so consumption is valued at the pooled
rate while slots may carry stale intra-batch detail (e.g. units
reconciled at zero and later merged). Consuming such a slot leaves a
negative value on positive qty. Spread the pool value across the
batch's slots when that happens; non-batchwise slots pool per
warehouse.
* fix: shift same-timestamp sibling SLEs when cancelling an entry
update_qty_in_future_sle compared against the reversal SLE's own
creation and skipped same-posting_datetime siblings on cancel, leaving
their qty_after_transaction stale and causing false negative stock
errors.
* fix: revert update_qty_in_future_sle cancel tie-break, it double-counted
Company.create_default_warehouses stored warehouse_name through the
session _(), so a site set up in a non-English language keeps its
default warehouses under translated names. The opening-stock fallback
in item.py then looks up {"warehouse_name": _("Stores")} from an
arbitrary session and misses the warehouse whenever the lookup
session's language differs from the creation session's.
Create the default warehouses with canonical English names, matching
the identity-_ convention install_fixtures uses for other default
records. The exists-guard also matches the session translation so a
re-run on a legacy site does not insert English duplicates next to
translated warehouses.
The "Stores" fallback lookup moves to get_stores_warehouse, which
tries the canonical name first and falls back to the session
translation so legacy sites keep resolving their translated warehouse.
The setup-wizard and test-bootstrap lookups drop _() since they now
run after English creation (install_fixtures already used identity _,
so its lookup silently missed translated warehouses before this
change).
Same bug class as #57345.
A name lookup misses roots created under a translated name (pre-#49875
setups) or renamed roots. Resolve the first parentless group by lft and
guard against self-parenting when the root itself is saved.
Native JSON request bodies deliver target_doc as a parsed dict, which the
str | Document type hints rejected, breaking every Get Items From button.
Unify all whitelisted mapper endpoint hints to str | dict | Document | None.
Needs frappe#41190 so get_mapped_doc converts the dict target.
_("All Item Groups") resolves in the session language, so for
non-English users the db.exists lookup missed the root (stored in
English) and new groups were saved parentless, becoming uneditable
second roots.
Closes#57345
Company.create_default_departments named and looked up the root
Department via _("All Departments"), which resolves in the session
language. A site set up in a non-English language stores the root
translated, and a company created later from a session in another
language misses it and inserts a second root, corrupting the tree.
Resolve the root once with get_root_of (falling back to the canonical
English name on fresh installs) and reuse it for the root record, the
exists-guard and the child departments' parent.
Also stop translating lookups of records install_fixtures stores under
English names: Price List "Standard Selling" and Print Headings
"Credit Note" / "Debit Note".
Same class as the root Item Group fix (#57386, issue #57345).
feat(shipping-rule): make cost center optional with company default fallback
Cost Center on Shipping Rule is no longer mandatory. When left blank, the
applied shipping tax row falls back to the company default cost center,
avoiding the 'Cost Center is required for Profit and Loss account' error on
submit. The rule's project is also applied to the tax row.
get_permitted_fieldnames never lists Table fields, so allowed_companies
cannot be asserted through it; the write-reset assertions already cover
that field.
The mt940 library exposes ``transaction_reference`` from the :20: tag,
which is the statement-level reference and identical for every
transaction in a statement. The bank-statement-to-CSV conversion was
using it verbatim, so every imported row ended up with the same
reference, making reconciliation impossible.
Read the per-transaction reference from ``customer_reference`` on the
:61: tag instead. Handle two edge cases:
- **Overflow >16 chars.** When a bank emits a single-line :61: whose
reference exceeds 16 characters, the mt940 regex splits the tail into
``extra_details``. Gate the rejoin to cases where
``customer_reference`` is exactly at the 16-char MT940 cap; below
that, ``extra_details`` is genuine supplementary information and must
not be appended.
- **``NONREF`` sentinel.** The MT940 standard marker for "no customer
reference". Check it against the un-concatenated value so that a
``NONREF`` customer reference with populated ``extra_details`` still
falls back to ``bank_reference`` instead of returning a junk
``NONREFsomething`` value.
Also switch the Description column to ``transaction_details`` (the :86:
tag content) so rows carry their real narrative instead of the mostly
empty :61: supplementary field.
Only the master-manager role of each doctype (Item Manager, Sales Master
Manager, Purchase Master Manager) can view and edit restrict_to_companies
and allowed_companies. Customer reuses its existing level-1 permission
rows; Item and Supplier get a new level-1 row.
test@example.com carries System Manager in the frappe fixtures, so on a
fresh CI site it can read Delivery Note and the lookup legitimately
returns the draft. test1@example.com has no roles, making the
no-permission assertion environment-independent.
set_route_options_for_new_doc lived in TransactionController, so doctypes
extending StockController directly (Stock Reconciliation, Stock Entry) missed
the Batch/SABB prefill or duplicated it locally. Move it to StockController
and call it from onload_post_render so all descendants inherit it.
- Batch quick entry from Stock Reconciliation items now prefills Item
- SABB route options unified: warehouse || s_warehouse || t_warehouse,
so transaction doctypes now also prefill warehouse
- Stock Entry's duplicate handler removed; its onload_post_render now
calls super
Meta.get_link_fields rescans the field list on every call; hoisting it
out of the row loop cuts the reference walk on a 100-row invoice from
2.8ms to 0.1ms.
Doctypes marked In Create (GL Entry, Stock Ledger Entry, Bin, ledger
entries) are system-created by definition, so derive their exemption
from meta instead of listing them.
Replace the manually maintained transaction allowlist with a wildcard
validate hook: any doctype carrying a Company link field is checked, so
new doctypes are covered automatically. System-managed doctypes (ledger
entries, reposts, bundles, bins, POS consolidation, bank feeds) are
exempted so cancel, repost and reconciliation of documents created
before a restriction changed keep working; that guarantee is pinned by
a cancel-after-restriction test.
Extend company restriction enforcement to the remaining user-entered
transactions (BOM, Work Order, Job Card, Production Plan, Pick List,
Blanket Order, asset and maintenance documents). Ledger and repost
doctypes stay excluded so cancelling or reposting older documents
keeps working after a restriction changes.
Restrict to Companies only filtered list views and document reads, and
only for users with Company user permissions. Any user could still use
a master restricted to Company A in a Company B transaction, and users
without Company user permissions bypassed the feature entirely.
Validate on save of transactions that every linked Item, Customer and
Supplier allows the transaction company, and filter item link queries
by the transaction company so restricted items don't show up in the
item selector.
frappe.get_list defaults to 20 rows; child-table joins can produce
duplicate parent names that fill the window and hide further drafts.
Also clarify the check-ordering regression test.
get_single_value inside _revalue_reconciled_batch_slots runs while
rows stream through the unbuffered cursor on MariaDB, killing the
active iterator. Resolve it once in generate() with the other
prefetches.
stock_value_difference / qty equals the new batch rate only when the
reco entry carries the entire batch, as the split out/in reco SLEs and
batches reconciled from zero do. Partial direct-batch_no entries mix a
qty delta with existing stock, so their slots keep prior values.
Plain items need no such guard: the valuation engine collapses the
FIFO stack to qty_after * valuation_rate on every reconciliation, so
rescaling remaining slots at the reco rate matches the ledger. Lock
that with a test.
Batch items take the batch-slot path, which mirrors the same value
arithmetic: the reco's incoming entry dumps the revaluation remainder
on one slot. Rescale each reconciled batch's slots at its post-reco
rate (stock_value_difference / qty of the incoming bundle entry).
A reconciliation's stock_value_difference includes the revaluation of
stock already in the FIFO queue, but the whole amount was attached to
the qty-delta slot while older slots kept pre-revaluation values. A
downward revaluation therefore produced negative bucket values in the
Stock Ageing report, and repeated recos let the queue total drift away
from Stock Balance.
Re-derive every slot value as qty * valuation_rate after processing a
reco SLE, since a reconciliation values the entire balance at its rate.
Covers both single-SLE recos and the zero-out/re-add pair that flows
through the transfer bucket.
* fix: show transaction currency symbol in Payment Request schedule dialog and reference table
When company currency (INR) differs from customer currency (USD), the Amount
column in the Select Payment Schedule dialog and the Payment Reference table on
the Payment Request form incorrectly displayed the company currency symbol (₹)
instead of the transaction currency symbol ($).
- Pass `currency` from the parent document on each schedule row returned by
`get_available_payment_schedules` so the dialog can resolve the symbol.
- Add a hidden `currency` field to the dialog table and set `options: "currency"`
on `payment_amount` so Frappe renders the correct symbol.
- Propagate `currency` into Payment Reference rows in `set_payment_references`.
- Add a hidden `currency` Link field to the Payment Reference child DocType and
set `options: "currency"` on its `amount` field so the table renders correctly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: preserve currency when serializing payment schedule rows
get_available_payment_schedules set `schedule.currency` directly on
the Payment Schedule Document row, but `currency` isn't a field on
that DocType, so the API response serializer stripped it before it
reached the client. The Select Payment Schedule dialog and the
Payment Reference table therefore always fell back to the company
currency symbol, even with the earlier options="currency" changes in
place.
Convert each row to a plain dict via as_dict() first, then set the
currency key on the dict so it survives serialization.
* refactor: source schedule currency in dialog instead of API serializer
get_available_payment_schedules had to convert each child row with
as_dict() and re-attach currency, because currency is not a field on
Payment Schedule and the response serializer drops attributes set on the
Document itself.
The schedule dialog already has the transaction currency on frm.doc, so
set it there and let the API keep returning the schedule rows unchanged.
Payment Reference still stores currency per row.
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Jatin3128 <jatinsarna8@gmail.com>
Co-authored-by: Jatin3128 <140256508+Jatin3128@users.noreply.github.com>
Stock Summary's sort selector only offered 5 of Bin's 10 qty fields; add
the rest (ordered, requested, planned, reserved for production plan,
reserved stock) and extend get_data's or_filters so bins whose only
nonzero qty is one of the new fields show up when sorted by it. Sort
labels now mirror Bin field labels.
Stock Projected Qty report had a column for every Bin qty field except
reserved_stock; add it.
When creating a follow-up document (SO->DN, PO->PR, PI->Payment Entry,
etc.), warn the user if a draft of the target doctype already linked to
the source document exists, with links to the drafts and the option to
proceed anyway.
The target doctype comes from the make_mapped_doc response via the new
frappe.model.add_mapped_doc_guard hook, so every open_mapped_doc flow is
covered without per-doctype code or method-name inference. The server
lookup walks parent-level and child-table Link / Dynamic Link fields of
the target doctype and queries through frappe.get_list, so role and user
permissions apply and docstatus filtering happens in the query itself.
Payment Entry creation bypasses open_mapped_doc, so its controller runs
the same guard explicitly.
refactor: clearer labels and messages, drop "threshold" wording
User-facing text only, no field or behaviour changes:
- Accounts Settings toggle label -> "Restrict Customer Over Billing".
- Bypass role label -> "Role Allowed to Bypass Over Billing Restriction".
- Customer Credit Limit field label -> "Overdue Limit".
- Rewrote the descriptions and the block message to match and to stop
saying "threshold".
* feat: inline serial and batch entries editor in Purchase Receipt
* feat: grid-style UX, deferred saves, scan and range options for inline serial batch editor
* feat: extend inline serial batch editor to all bundle doctypes with auto fetch
* fix: address review comments on inline serial batch editor
* fix: escape untrusted values in inline editor alerts
* fix: clear child bundle reference only when the row owns the bundle
* fix: keep inline serial batch editor disabled on existing sites via patch
Refreshing a single grid row only updates the active row, so the derived amount
for rows after the edited one went stale. Recompute every row's amount from
qty x rate and re-render the grid.
- Proforma Invoice form: place Grand Total beside Total Quantity (column break)
and minor field reorder
- Rename the tab button to "New Proforma Invoice"
- Add the Proforma Invoice client-script scaffold
- Move the Proforma Invoice settings section from the Subcontracting Inward tab
to the Transaction tab
- List Proforma Invoice in the Document Naming tab so its naming series can be
configured there
Replaces the Global Defaults toggle. Each Item/Customer/Supplier now
carries a Restrict to Companies checkbox: the Allowed Companies table
only shows (and is mandatory) when checked, is cleared on uncheck, and
permission filtering, read denial and write validation apply only to
masters that have the checkbox set.
get_stock_balance_for fetched serial nos across every batch in the
warehouse, so reconciling one batch of a serial+batch item compared the
selected serials against the pool of all batches and failed whenever
multiple batches existed.
get_mapped_doc copies same-named fields by default. work order item's
transferred_qty (cumulative across the whole work order) was leaking into
the new pick list item's transferred_qty (meant to track how much of
that pick list row has been converted into a stock entry, starting at 0).
the leaked value then got subtracted again in
get_pending_transfer_stock_qty(), so every pick list after the first
under-transferred raw materials by whatever was already recorded on the
work order, driving material_transferred_for_manufacturing towards zero
across repeated partial pick-list/finish cycles.
fixes#57236, related to #56596
Repost Item Valuation and BOM Update Log cleared old logs with a raw
delete on the parent table, orphaning timeline comments, versions,
attachments and other reference records.
Fixes#57237
update_item_rates passed price_not_uom_dependent, a key
get_price_list_rate_for never reads, and omitted conversion_factor, so a
stock-UOM price was never converted to the row UOM. The function's
(historically misnamed) price_list_uom_dependant ctx key carries the
Price List's price_not_uom_dependent value: truthy returns the found
rate as-is, falsy multiplies by conversion_factor.
Also guard on_update with is_new(): has_value_changed returns True when
there is no doc_before_save, so every first save re-wrote item rates.
Group fields with section and column breaks: Details (two columns), Items with
right-aligned totals, Print Settings (two columns), and Status (two columns).
* feat: block sales invoice submit when customer overdue exceeds threshold
Adds an opt-in, per-customer Overdue Billing Threshold. When enabled in
Accounts Settings, submitting a Sales Invoice is blocked if the customer's
overdue amount exceeds their threshold, unless the current user holds a
configured bypass role. Modeled on the existing credit limit feature.
- Accounts Settings (Credit Limits tab): enable toggle + bypass role.
- Per-customer threshold on the Customer Credit Limit table, shown only
when the feature is enabled via a property setter (same mechanism as
subscription / accounting dimension sections). Table relabeled to
"Credit & Overdue Limits".
- Overdue is read live from the ledger via get_outstanding_invoices
(payments already netted), summing Sales Invoices past their due date.
- Enforced in Sales Invoice on_submit, after the credit-limit check;
returns are exempt.
- validate_credit_limit_on_change no longer trips when a row sets only
the overdue threshold (credit_limit = 0).
Fixes#52960
* fix: compute overdue amount in company currency and format with fmt_money
get_customer_overdue_amount now sums GL Entry debit - credit grouped per
invoice, which is always booked in company currency, instead of using
get_outstanding_invoices which returns the receivable-account currency.
The threshold is in company currency, so the previous comparison could mix
currencies for customers with a foreign-currency receivable account. This
mirrors how get_customer_outstanding computes the figure for the existing
credit-limit check.
The blocking message now formats both amounts with fmt_money using the
company currency.
Adds a test asserting a 100 USD invoice at a conversion rate of 50 is
counted as 5000 in company currency.
* refactor: drop redundant threshold coercion and dead test cleanup
- Coerce the overdue threshold with flt() once when reading it, instead of
calling flt() on it at each of the three use sites.
- Remove a no-op set_overdue_billing_threshold() call in the feature-disabled
block (the threshold was already set to that value) and the trailing reset,
which is dead since each test is rolled back.
No behaviour change.
* fix: compute overdue amount from payment terms, matching the Overdue status
The overdue amount keyed on Sales Invoice.due_date, which set_due_date() sets
to the LAST payment term. An invoice whose first term was past due and unpaid
was therefore counted as zero, even though ERPNext already shows it as Overdue
in the invoice list. The gate and the UI could disagree.
get_customer_overdue_amount now follows the same rule as is_overdue(): per
invoice, the amount that has fallen due (sum of payment schedule terms past
their due date) minus what has been paid, clamped to the outstanding balance.
Invoices without a schedule (POS, opening) still fall back to the invoice due
date, mirroring is_overdue()'s own guard.
The ledger stays the source of truth for what is unpaid: the outstanding per
invoice is still SUM(debit) - SUM(credit) from GL Entry. base_payment_amount is
always stored in company currency, so no currency conversion is needed and the
comparison against the threshold stays consistent.
Adds a test covering a two-term invoice: only the past-due term counts, and
paying it off clears the overdue amount.
* feat: honour the overdue billing threshold set on the customer group
The threshold lives on Customer Credit Limit, which is also rendered on
Customer Group. A threshold set there was stored but never evaluated, so the
configuration was a silent no-op.
get_overdue_billing_threshold now reads the customer's row and falls back to
its customer group, mirroring get_credit_limit. The group's
bypass_credit_limit_check is deliberately not consulted: it is labelled for the
credit limit check at sales order and is unrelated to overdue billing.
get_customer_group_details also dropped the threshold when copying group rows
onto a customer, because it copied a single hardcoded field per table. It now
copies a list of fields per table, so credit_limit and overdue_billing_threshold
both carry over.
Add a "Hide Item Quantity in Print" option (Amount basis only) that omits the
qty and rate columns from the printed proforma, for a clean value-based
document that shows only item and amount.
In Amount basis, both qty and amount are now user-entered and the rate is
derived from them (rate = amount / qty). Previously qty was forced to the
ordered qty, which ignored an edited qty when switching basis.
- Pre-fill each line with the remaining (ordered minus already proformed) qty
and amount, so the default no longer trips the excess warning
- Place the Proforma Invoice action after the standard Create options
The work_order link filter in Stock Entry passed the string
`tabWork Order`.produced_qty as a filter value. It was never a real
column comparison: db_query coerces string values on numeric fields
with flt(), so the condition silently degraded to qty > 0, and on
backends that don't coerce text to numeric (postgres) such filters
fail with InvalidTextRepresentation.
Move the condition into a whitelisted search query that compares the
columns properly (qty > produced_qty), mirroring pick_list's
get_pending_work_orders.
Show a non-blocking notice below the item table when a line's total proforma
quantity (or amount) — this proforma plus already-issued ones — exceeds the
Sales Order line's ordered value. It updates live as the qty/amount or the
basis changes, and the user can still create the proforma.
Issued-proforma qty/amount are aggregated per line on demand (cancelled
proformas excluded); nothing is stored on the Sales Order.
set_transit_warehouse re-tested from_warehouse inside a guard that
already requires it, so the Company branch of its ternary was
unreachable: with no default on the source warehouse the field stayed
empty and Company.default_in_transit_warehouse was silently ignored.
Try the warehouse's default first, then the company's.
Cancelling a proforma should void it, not erase history.
- Persist the Cancelled status on cancel (db_set) and keep the PDF attached
- Proforma tab now lists cancelled proformas with a red status badge, so the
voided document and its PDF stay reachable for audit
Let a proforma be created by editing item amount instead of quantity, for
value/advance-style proformas.
- "Based On" (Quantity | Amount) on the proforma and the create dialog
- Amount basis keeps the ordered qty and derives a rate so the line totals
the entered amount; the PDF renders the same in-memory Sales Order copy
- Proforma Invoice Item now stores rate and amount
Remove the pending/proforma-qty machinery: it only fit staged, incremental
proformas and misrepresented the common whole-order / re-issued cases.
- Drop proforma_qty from Sales Order Item and its submit/cancel write-back
- Drop pending-qty aggregation and the over-qty soft warning
- The create dialog now pre-fills the ordered qty (editable down)
Cover partial proforma being non-blocking on delivery/billing,
pending-qty aggregation with cancelled proformas excluded, tax scaling
to the partial qty, over-qty as a soft warning, and the settings gate.
- Create > Proforma Invoice dialog with naming series, print format and
letter head selectors, and an item-wise pending-qty grid
- Proforma tab listing issued proformas with inline view/email actions,
shown only once at least one proforma exists
- Register the client script and add the connections dashboard link
Workspace re-export in #56864 duplicated every link in the Home and
Projects workspaces, so desk renders each link twice. Same issue as
55afd95b20. Bumped modified so existing sites re-sync.
- Submittable, non-accounting Proforma Invoice + Proforma Invoice Item
child doctype (in_create; posts no GL/stock, stores item + qty only)
- Server API: pending-qty aggregation per Sales Order line (issued
proformas only), make_proforma_invoice (sole creation path, gated on
the settings toggle), PDF rendered from an in-memory qty-adjusted copy
of the Sales Order and attached, send_proforma_email
- Non-blocking proforma_qty write-back to the Sales Order on submit/cancel
* feat: book Expenses Added To Stock GL entries for Stock Entry, Stock Reconciliation and LCV
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: make stock expense GL booking configurable via Accounts Settings
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: skip stock expense booking for unconfigured companies, check flag once per compose
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The transfer flow ignored Consider Minimum Order Qty twice: the JS
handler force-reset the checkbox before fetching items, and the
purchase remainder left after allocating transfers from other
warehouses was never raised to min_order_qty (the check runs on the
total requirement before the split).
Drop the JS reset and apply min order qty to the purchase remainder,
in stock UOM before the purchase UOM conversion.
Two simultaneous allocations for the same item can both claim the same stock
on postgres: the picked-items locking read cannot see the rows another
in-flight creation is inserting, while MariaDB's gap locks make the creations
take turns. Advisory-gate set_item_locations per item (sorted against
deadlocks) so the second allocation waits, then subtracts the first's claim.
MariaDB unchanged.
Same hasattr pattern as repost_gate: an ERPNext ahead of its frappe build keeps
the status-quo serialization-failure retries instead of failing every stock
submission on postgres.
The for_update read in _ensure_idle_system only blocks new GL inserts on
MariaDB, via the gap lock it takes; a postgres row lock never blocks inserts,
so the guard silently degraded to the 5-minute recency check. LOCK TABLE IN
EXCLUSIVE MODE blocks writers (not readers) until the rename commits and NOWAIT
keeps the wait=False fail-fast, feeding the existing QueryTimeoutError path.
Postgres locking reads never see rows a concurrent transaction is inserting
(MariaDB's gap locks block the insert, then its locking reads return the fresh
row), so two concurrent writers for the same (item, warehouse) compute from the
same stale previous SLE and the loser overwrites Bin with a wrong absolute qty.
Today only the REPEATABLE READ serialization-failure retry catches this; the
gate makes correctness lock-based, covers the empty-history first-transaction
case (nothing exists to row-lock), and keeps negative-stock validation accurate
against concurrently inserted SLEs. Taken at the top of make_sl_entries (sorted
pairs, before the future_sle_exists cache warms) and in
update_entries_after.__init__ for the repost paths; re-entrant, released at
commit. MariaDB paths unchanged.
* fix: permission check for `get_task_html` and `get_timesheet_html`
* fix(project): enabled project access control for users without `Projects User` Role
* fix(portal): validate user permissions for project portal
* fix: patch to add docshare for the project users
* fix(patch): selecting correct column on the query
* fix(project): grant access to all the current users for new project
* fix(portal): fixed condition to display timesheets on project
* test(portal): add access control tests for project user
* fix(project): using `frappe.has_permission` instead of `self.has_permission` to validate user permissions
* fix(project): granting docshare access for every ProjectUser
Roles for an User can be removed any time or an User Permission can be added which might restrict the access to the Project.
* fix(patch): create docshare documents for non-cancelled projects and users who have no docshare documents
* test(project): removed `test_control_access_does_not_touch_users_with_real_permission`
* fix: skip redundant reposting of dependent items
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: use earliest cascade datetime and batch repost item lookup
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix: name every conflicting voucher in the reserved batch error
* fix: exclude fully-delivered reservations from the conflict message
* fix: round outstanding qty guard consistently with the conflict gate
validate_reserved_batches compared the voucher's own qty against the
remaining batch qty, so delivering one order's reserved unit threw
Reserved Batch Conflict whenever the remainder exactly matched another
order's reservation. Compare the remaining batch qty against the
aggregated outstanding reserved qty (qty - delivered_qty) of other
vouchers instead, excluding reservations the voucher itself delivers.
erpnext.accounts.dashboard_fixtures and erpnext.buying.dashboard_fixtures
were removed in 2020 when dashboards were exported to JSON fixtures.
The assets module's dashboard_fixtures.py was left behind unreferenced;
its dashboard, charts and number cards already exist as exported JSON.
StockReservation.transfer_reservation_entries_to() created the transferred SREs without copying stock_uom, in both the entries_to_reserve dict and the extra-items fallback. get_items_to_reserve() already selects the item's stock_uom, so entry.stock_uom is used.
On sites with a global default stock_uom (e.g. "Nos"), frappe's _set_defaults() backfilled the blank field, so the transfer silently stored the wrong UOM for any item whose stock UOM is not the default. On sites without that default the SRE's validate_mandatory() raised "Stock UOM is required", aborting Work Order submission for the Subcontracting Inward Order / Production Plan flows.
(cherry picked from commit 5991ecfa3d)
make_all_scorecards' dedup query used strict bounds, so a single-day
period (supplier created on a month's last day -> start == end under
"Per Month") never matched its own window and was re-created on every
call. The daily refresh_scorecards job would insert a duplicate
submitted period each day for such suppliers, and
test_make_all_scorecards_is_idempotent fails on any date where
nowdate() - 75 days lands on a month end — both nightly server suites
went red on 2026-07-14 (75 days after April 30).
Inclusive bounds cannot false-match adjacent periods: each next period
starts at end_date + 1, so closed intervals never touch.
covers sales order, quotation, sales invoice, delivery note and
purchase order, asserting actual_qty from the row warehouse and
company_total_stock across all warehouses of the company.
company was passed to get_bin_details only for purchase order, so
company_total_stock was never returned for sales order, quotation,
sales invoice and delivery note and the qty (company) column always
read zero. pass ctx.company for every doctype, which also drops the
dependency on doc being supplied.
on the client, set_actual_qty copied only actual_qty out of the
response, so qty (company) never refreshed on a warehouse change. use
frm.call with child so every bin field is applied, pass
include_child_warehouses to match the server, and include quotation.
- allow new rows on scan when pick manually is enabled, since only
then are scanned rows not subject to being overridden by
set_item_locations on save
- stop capping picked qty at the default demand qty (1) for rows
added by the scanner itself, so repeat scans of the same barcode
keep incrementing the row instead of failing with "maximum
quantity scanned"
- ignore barcode uom when matching an existing row if new rows
aren't allowed, since there's no alternate-uom row to fall back to
Seeding a current-dated USD->INR rate makes get_exchange_rate resolve
62.9 on today() instead of hitting the live API, which exposed three
tests that implicitly relied on a different/undefined current rate:
- customer: dropped its own colliding current-dated seed (ignored via
ignore_if_duplicate, and its cleanup deleted the shared seed) and now
asserts the quotation resolves the seeded rate via get_exchange_rate.
- exchange_rate_revaluation: the revalued rate (62.9) is now below the
booked 80, so the revaluation is a loss (debited) rather than a gain;
derive the gain/loss column from the sign instead of assuming a gain.
- purchase_invoice: the receipt rate was an accidental tuple (70,) that
got discarded and recomputed to the seed; set explicit rates with the
receipt above the invoice so the stock exchange difference is a credit,
matching the asserted column.
#56871 routed any searchfield without a DocField meta — including
"name", which get_search_fields() always appends — into the
`= cint(txt)` branch. For non-numeric search text cint() yields 0, so
`is_group = 0` (Cost Center) matched every leaf record on both
engines, and `name = 0` matched every non-numeric name on MariaDB.
Skip Check fields from or_filters entirely — a checkbox can't match
search text — and keep LIKE for everything else, including "name".
* feat: explain FIFO allocation of fixed Discount Amount on Sales Order (#56436)
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Diptanil Saha <diptanil@frappe.io>
(cherry picked from commit 62fed1d562)
# Conflicts:
# erpnext/selling/doctype/sales_order/sales_order.json
* chore: resolved conflicts
---------
Co-authored-by: Mohammad Umair Sayed <umair_sayyed@yahoo.com>
Co-authored-by: Diptanil Saha <diptanil@frappe.io>
Fetch job_card_item for all pick list locations in one query instead
of one per row, and prefer the job card's semi_fg_bom over the work
order BOM, mirroring the direct Job Card -> Stock Entry mapper.
hoist the invariant work order lookup and batch-fetch work order item
source warehouses once instead of querying per raw material row in
get_bom_raw_materials
A Stock Entry created from a Pick List against a job card's Material
Request never set job_card, job_card_item, fg_completed_qty or the
'Material Transfer for Manufacture' purpose, so the Job Card did not
recognize the transfer and blocked submission. The WIP warehouse was
also not populated.
Route such pick lists through a job-card-aware branch mirroring the
direct Material Request -> Stock Entry mapper, and set the purpose to
'Material Transfer for Manufacture' in the work order branch so the
WO -> MR -> Pick List flow updates the work order too.
* fix(stock): recompute moving average item slots
* test(stock): add test to validate the stock value of moving average items
* fix(stock): support lifo valuation in stock ageing report
lifo items were aged as fifo (oldest consumed first), so the report kept the
newest lots on hand and reported the wrong stock value and average age. prefetch
each item's valuation method (it can't be resolved mid-stream without breaking the
unbuffered cursor) and consume from the tail for lifo items. also reuse that shared
lookup in the moving average revaluation pass. scoped to plain items; batch, serial
and same-voucher repack legs stay on fifo.
* test(stock): add test for lifo consumption in stock ageing report
verify child-stock aggregation, transfer sourcing from child warehouses,
that a for warehouse outside the group is rejected, and that a for
warehouse is required when raw materials are fetched.
add an optional raw material group warehouse on production plan. when set,
raw material availability is checked across its child warehouses (bin rows
aggregated), while material is still received into for warehouse. for
warehouse is restricted to a child of the group and required when raw
materials are fetched; a group warehouse can never reach a material request
line. when the group is left blank, availability falls back to for warehouse
and the previous flow.
the whitelisted endpoint typed doc as str | frappe._dict | Document, so a
json request body (a plain dict) failed pydantic type validation. widen to
str | dict | Document, matching the convention used elsewhere in erpnext.
The stock_entry_handler modules were moved to services; update the
retention, expired-batch and subcontract call paths in the client
so the whitelisted methods resolve again.
The type-hint refactor rejected the doc dict sent by the form
controller; broaden the annotation to match ctx and fix the
cts= kwarg typo in transaction_base.
Tests that create USD documents dated today() (e.g. Sales Order in
test_advance_payment_ledger_entry, USD BOM in test_routing) rely on
get_exchange_rate() finding a USD->INR Currency Exchange record. The
only seeded records are dated 2016, so the lookup misses and falls back
to an external API that is blocked in CI, returning 0. That surfaces as
"Exchange Rate is mandatory" on Sales Order validation and a
ZeroDivisionError in BOM.get_routing (hour_rate / conversion_rate).
Whether it passes depends on which shard incidentally committed the
2016 records first, making it an order-dependent flake that unrelated
PRs trip by shifting test distribution.
Seed today()-dated USD<->INR rates once in BootStrapTestData so the
lookup resolves deterministically without the external API. Rates
mirror the latest Currency Exchange test_records to keep cost
calculations unchanged.
The routing field handler only fetched operations from the routing when
the operations table was empty. When a new BOM version is created (via
"New Version"), operations are copied from the source BOM, so selecting a
different routing left the old operations in place - both in the form and
after saving.
Drop the `!frm.doc.operations.length` guard from the routing handler so
that (re)selecting a routing always refetches the operations from that
routing via the existing get_routing method, which clears and repopulates
the operations table.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Continues the AccountsController service decomposition (Phase 5).
- Add accounts/services/deferred_accounting.py with DeferredAccountingService
owning the deferred revenue/expense validations (income/expense account
defaulting and service start/end date checks).
- Move the document-schedule orchestration (validate_all_documents_schedule
and the invoice/non-invoice variants) into PaymentScheduleService, where
they already delegated, removing the controller-to-service round trip.
- Update the three validate() call sites; keep
validate_auto_repeat_subscription_dates on the controller (still called by
buying/selling controllers).
No behavior change. accounts_controller.py 1818 -> 1745 lines.
* fix(stock): fall back to current date/time for serial and batch bundle posting datetime
Pick List has no posting_date/posting_time fields, so creating or updating a
Serial and Batch Bundle from a Pick List row crashed with
"TypeError: combine() argument 1 must be datetime.date, not None". Fall back
to today/now when the parent voucher doesn't carry its own posting date.
Fixes#56951
* fix(stock): accept a plain dict for add_serial_batch_ledgers' doc and child_row
The whitelisted add_serial_batch_ledgers only converted child_row into an
attribute-accessible frappe._dict when it arrived as a JSON string, and doc's
type hint only allowed Document | str. Frappe's JSON API delivers both as
plain dicts (see frappe.app.make_form_dict, which parses the request body
with orjson and only wraps the top-level dict, not nested values), so every
real request was rejected before the handler body ever ran: first with a
FrappeTypeError on doc, and once that's fixed, with an AttributeError on
child_row.serial_and_batch_bundle. parse_json already wraps a plain dict in
frappe._dict (and leaves a real Document instance untouched), so routing
child_row through it unconditionally fixes both.
* feat(manufacturing): create material request for raw materials from work order
allow raising a material transfer request directly from a work order,
mirroring the existing job card flow, so stores can fulfil it into wip
before the actual stock entry happens
* test(manufacturing): cover work order material request flow
verify the material request created from a work order carries the
right bom/purpose onto the resulting stock entry, and that the work
order status still moves to in process on a partial material-request
transfer
Remove per-doctype branching from the shared stock GL composer so each
voucher owns its own behavior:
- Move the Stock Reconciliation voucher-detail synthesis out of
BaseStockGLComposer.get_voucher_details into a
StockReconciliationGLComposer.get_voucher_details override.
- Replace the hardcoded doctype allow-list in check_expense_account with
an overridable class attribute enforce_pl_expense_account (default
True). Vouchers that post the difference to a balance-sheet account
(Stock Entry, Stock Reconciliation, Delivery Note) set it False.
- Add DeliveryNoteGLComposer to own the P&L-exempt rule and wire
DeliveryNote.get_gl_entries to it.
No change to GL output; behavior is relocated, not altered.
* fix: added permission checks on various whitelisted functions
* fix: permission checks on `get_party_account` and using "select" `ptype`
* test(`BootStrapTestData`): assign `Accounts User` role to User `test@example.com`
update_current_stock() in delivery_note.py used to call
frappe.db.get_value("Bin", ...) separately for every row in items and
every row in packed_items - so a delivery note with 200 items and 200
packed items made 400 separate database calls on every save.
now it groups item codes by warehouse and fetches bin data with one
query per distinct warehouse, then assigns actual_qty/projected_qty to
each row from that result - same values as before, far fewer database
calls, and no cross-product over-fetch across warehouses.
get_item_warehouse_projected_qty ran an uncached frappe.get_doc per Bin
row to walk the warehouse parent chain, re-fetching the same ancestors
for every item sharing a warehouse. Preload the warehouse parent map
once and walk it in memory instead.
based_wise_columns_query() and group_wise_column() in trends.py built
column labels as raw strings, so "Item", "Item Name", "Customer",
"Supplier", "Territory", "Currency", etc. never went through _() and
stayed in English regardless of the user's language, unlike the
period and total columns right next to them which were already
wrapped correctly.
Add regression coverage for the new abbreviation-rename propagation:
a simple item_code rename, item_name derived from a template whose
item_name differs from its item_code, and a manually customized
item_name getting rebuilt rather than left stale.
Item Attribute abbreviations only got baked into a variant's item_code
and item_name at creation time (make_variant_item_code returns early
once item_code is set). Renaming an abbreviation afterwards left every
existing variant stuck with the stale code, silently out of sync with
its own attribute.
Detect abbreviation renames on Item Attribute save, find every variant
using the affected value, and rebuild+rename its item_code via
frappe.rename_doc so linked records follow along. item_name is rebuilt
in lockstep from the template's item_name, even if it had since been
customized, since both fields are meant to be derived from the same
abbreviation.
On postgres, outward batch valuation row-locked the item's ENTIRE SLE / Serial and Batch Entry history (a separate plain SELECT ... FOR UPDATE per site, since FOR UPDATE is invalid with GROUP BY there). That writes a lock marker (xmax + WAL) on every historical tuple per outward movement - write amplification that grows with history forever - and locks nothing at all when the history is empty (negative-stock edge: two concurrent outwards don't serialize).
Replace all four history-wide postgres lock statements with one frappe.db.transaction_advisory_lock(("batch-valuation", item_code, warehouse)) at the top of BatchNoValuation.calculate_avg_rate's outward branch - every valuation read (bundle + the three deprecated paths) is downstream of it. Released at commit/rollback, participates in deadlock detection, and serializes regardless of history size, so the empty-history edge is closed by construction. Batch qty updates iterate in sorted order so concurrent vouchers lock Batch rows in the same sequence.
MariaDB is unchanged: it keeps the original grouped FOR UPDATE row locks (and its gap locks). Requires frappe#40621.
Test: outward delivery of a batched item must leave the xact advisory lock visible in pg_locks for the submitting transaction.
Postgres has no gap locks, so the lock-then-read pattern (plain SELECT ... FOR UPDATE before a grouped read) only serializes on rows that already exist. Two sites had reachable races where the lock set is empty or disjoint:
- Pick list: two pick lists against the same SO item submitted concurrently lock only docstatus=1 rows, so with no previously-submitted picks their lock sets are disjoint and both pass validate_picked_qty (over-pick; picked_qty last-writer-wins). Gate on the referenced Sales Order Item / Packed Item rows, which always exist.
- Stock reservation: the first concurrent reservations for an (item, warehouse) find no SRE rows to lock, so both pass and reserved qty can exceed actual. Gate on the Bin row, which exists once there is stock.
MariaDB is unchanged (its gap locks already serialize both; the gates are postgres-only). Also: ORDER BY on the small-set postgres lock selects for deterministic lock order, and the repost pre-lock in get_future_stock_vouchers selects a constant instead of shipping every matching SLE name to the client.
A full audit of the loose-GROUP-BY fixes found four recurring mistakes in the fixes themselves: incoherent Max/Min pairs over coupled columns, NULL-skipping Max on discriminators, Sum(x)*Max(y) fabricated arithmetic, and wrong-bound picks. Add them to the compatibility catalog (new 3.1) and to the Greptile review instructions so future PRs get flagged.
The Budget Variance Report chart plotted the actual expense one month
earlier than the table (e.g. July actual shown under June).
build_comparison_chart_data() collected budget columns using
fieldname.startswith("budget_"). The dimension column "budget_against"
also matches that prefix, so it was added as an extra leading entry to
budget_fields and labels, while actual_fields had no such leading entry.
This shifted every actual value one position ahead of its label.
Skip the "budget_against" dimension column so budget/actual values and
labels stay aligned per month.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
With an explicit GROUP BY, a zero-match detail query returns no rows (the old bare aggregate returned one all-NULL row), so row1[0][0] would raise IndexError. Skip the entry instead.
When a Material Request lists the same item on multiple rows, the consolidated row showed Max(schedule_date), understating urgency. Use Min - the earliest date the item is needed.
arrival_qty summed the all-time ordered qty of every submitted PO line for the item+warehouse, so it grew monotonically with purchase history. Sum the pending qty (qty - received_qty) on open POs instead, and take the earliest schedule date from the same scope.
get_requested_amount multiplied the pooled pending qty of all matching Material Request items by a single Max(rate), fabricating the total whenever rates differ and biasing it upward - making false Budget Exceeded stops more likely. Sum (stock_qty - ordered_qty) * rate per row instead, matching get_ordered_amount.
get_po_entries aggregated every non-key column with Max() over (PO, material_request_item), which could stitch values from different PO lines into a row that never existed (one line's item_code with another's qty and amount). Select one representative line per group instead: a subquery picks Min(child.name) per group under the same filters and the outer query reads all columns bare from that line. Row count is unchanged.
When a BOM lists the same item twice (one line phantom via sub-BOM P, one non-phantom via sub-BOM N), grouping by item_code and aggregating bom_no and is_phantom_item with independent Max() could pair one line's phantom flag with the other line's bom_no, exploding the wrong sub-BOM or silently dropping a direct requirement. Group by the pair instead so each line keeps a coherent (bom_no, is_phantom_item); the consumer accumulates duplicate keys and explodes phantom rows with their own qty. Same fix as 41da9eb7fc, which missed this single-level path.
When a Sales Invoice is in a foreign currency (e.g. USD) but the receivable
account is in the company currency (e.g. INR), `outstanding_amount` on the
invoice is stored in the party account currency (INR). `postprocess_dunning`
was copying that value directly into the Dunning's Overdue Payment row, which
is expected to carry the transaction-currency (USD) amount.
The fix: when `party_account_currency != currency`, use
`payment_schedule[0].outstanding` (already maintained in transaction currency)
instead of `outstanding_amount`.
Closes#56006
* feat: shop floor interface for operators
* fix: documentation
* fix: UI/UX for shop floor
* fix: shop floor query and OEE edge cases from review
- Push the draft / To Manufacture condition into the Job Card query
(or_filters) so a busy workstation's submitted history cannot fill
the row limit and hide active drafts
- Clamp the OEE quality factor at zero when process loss exceeds
completed qty
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Add methods=["POST"] to 50 whitelisted functions that create or modify
documents (get_doc followed by insert/save/submit), so they can no
longer be invoked via GET requests.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Process Period Closing Voucher and Process Period Closing Voucher
Details are trackers how the jobs are processed. Keep transactions on
them very short.
- Update using child table name to avoid scanning whole table, which
eventually leads to mariadb 1020 (REPEATABLE READ).
- Avoid race condition in final summarization
the existing test_cost_center_for_manufacture only checks a raw material
row against an item-level override, which is set independently of the
":company" default guard and never exercised the bug.
the ":company" default pre-filled every row before set_default_cost_center()
ran, so its "if not row.cost_center" guard was always false and the
project/item group/brand priority chain in get_default_cost_center()
never ran.
- restore mutated SLE after test via addCleanup
- explicit return False in has_difference
- comment the fifo_stock_diff guard for non-queue predecessors
- 'Show Incorrect Entries' always returned an empty result (regression
from #43619); now returns entries from one row before the first
incorrect one
- FIFO queue columns were computed for serialized/batched SLEs that
don't maintain a stock queue, showing false differences; left empty
for such rows
- compare value/valuation differences at currency precision, qty at
float precision
Cover the pick-list flow where a stock entry moves only one of the work
order's required items: material_transferred_for_manufacturing stays 0 (min
fraction) while the status must move to "in process".
A stock entry created from a pick list has fg_completed_qty=0, so
material_transferred_for_manufacturing is derived from the min-fraction of
item-level transfers. When a pick list moves only some required items, the
un-picked item stays at 0, which zeroes the aggregate and leaves the work
order status at "not started" even though material is already in wip.
Promote the status to "in process" when any raw material has been transferred
via a pick list. material_transferred_for_manufacturing stays min-fraction
based (0 correctly means no full finished good can be started yet).
`get_work_orders` bounded a BETWEEN on the datetime columns `creation`
and `actual_end_date` with a bare date `to_date`, which MariaDB coerces
to midnight. Work orders created after 00:00:00 on the period's last day
were therefore dropped from the report (and made the new coverage test
fail on month-end CI runs). Extend `to_date` to end of day.
* feat: add company setting to enable Stock Delivered But Not Billed accounting
* test: add tests for Stock Delivered But Not Billed account config
* fix(company): skip outstanding SDBNB validation when no previous config exists
* test: add dedicated company fixture for SDBNB tests
* test: use SDBNB company for Sales Invoice SDBNB test
---------
Co-authored-by: Pugazhendhi Velu <pugazhendhi720@gmail.com>
Co-authored-by: Pugazhendhi Velu <126157273+PugazhendhiVelu@users.noreply.github.com>
Reversing a submitted Journal Entry opened a draft with reversal_of set,
which called frm.set_read_only(). That strips the write and submit perms
from frm.perm, so the toolbar never rendered the Save (or later Submit)
button and the reversal could not be saved.
Lock the fields and the accounts grid as read_only instead, leaving perms
intact so Save and Submit still work while nothing stays editable.
Ticket: 72857
MariaDB falls back to the existing deadlock-retry path; the advisory-lock
serialization from #56697 now applies on Postgres only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An incoming SLE without resolvable serial/batch details hit the
negative-head branch in _compute_incoming_stock even when the head was
a batch slot, because flt() on the batch number string returns 0.0.
_add_to_negative_fifo_head then crashed with
"TypeError: can only concatenate str (not 'float') to str".
Guard the branch with is_qty_slot, mirroring the existing check in
_add_transfer_slot_to_fifo_queue.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: validate reverse GL entries on current date under immutable ledger
When Immutable Ledger is enabled, the reverse GL entry is posted on the
current date, but the closed-period checks in make_reverse_gl_entries still
validate against the original (backdated) posting date. This blocks cancelling
a backdated voucher, such as a suspense Journal Entry for a migrated NPA loan,
with a books-closed error even though the reverse entry lands in an open period.
Validate both check_freezing_date and validate_against_pcv against the current
date when Immutable Ledger is enabled. When it is disabled, behaviour is
unchanged.
Follow-up to #55268.
* test: reset frozen till date after reverse entry test
The freeze date set on the company was not reset, so it leaked into the next
test which posts entries in that period. Reset it in a finally block.
* fix: prefer explicit posting_date under immutable ledger
Prefer the posting_date argument before frappe.form_dict and getdate, at both
the validation and the GL entry site, so an explicit date passed by the caller
is honoured and validation still matches the posted date.
repost() decided whether a failed Repost Item Valuation was recoverable
(re-queue as "In Progress") or permanently "Failed" by string-matching the
traceback for "timeout" or MariaDB's "Deadlock found". On Postgres a deadlock
surfaces as "deadlock detected" / "could not serialize access" and matches
neither, so a retriable deadlock was marked Failed and never re-queued -- the
scheduler only re-picks Queued/In Progress entries.
Classify by isinstance(e, RecoverableErrors) instead, the same tuple already
used to gate the error email. This covers deadlocks and lock/query timeouts on
both engines (frappe raises QueryDeadlockError / QueryTimeoutError uniformly)
and the advisory-lock repost gate's own QueryTimeoutError, which previously
recovered only because its class name incidentally contains "timeout".
- REPOST_LOCK_TIMEOUT 600 -> 300s: a contended waiter re-queues and frees its
long-queue worker slot sooner instead of pinning it for up to 10 minutes
(still well under the 1800s repost job timeout).
- Collision-free lock key: pass a ("stock_repost", item, warehouse) tuple so a
colon in item_code/warehouse can't map two distinct pairs onto one lock.
- Document that the gate is repost-vs-repost only; the synchronous
repost_current_voucher submit path is deliberately left ungated (gating a live
submit behind a background repost would be a worse regression).
Postgres-guarded on_doctype_update indexes: partial WHERE is_cancelled=0 + covering INCLUDE on GL Entry/SLE and Serial and Batch Bundle/Entry, and pg_trgm GIN on Item item_code/item_name (~128x faster LIKE search at scale). No-ops on MariaDB. Requires frappe framework support.
Address review feedback:
- A typed-but-not-selected value passed validation yet was dropped by
get_selected_attributes (reads committed pills only). Treat any pending
input as an error so it is never silently omitted from creation.
- Escape pill / pending values before interpolating them into the HTML
error message.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 'Create Multiple Variants' dialog rendered one checkbox per attribute
value and read the numeric config from the variant attribute child row. This
broke in several ways:
- A template whose attribute was made numeric after being added kept
numeric_values=0 on the child row, so the dialog treated it as non-numeric,
queried the empty Item Attribute Value table, and showed no values.
- Enumerating a large range (e.g. 1-100000) into checkboxes froze the browser.
Rework the dialog:
- Read numeric_values / from_range / to_range / increment from the Item
Attribute master, and guard increment > 0.
- Replace the checkbox-per-value list with one MultiSelectPills per attribute,
with a search placeholder.
- Stop enumerating numeric ranges: preview the first few values and validate
typed input against the range on demand, so huge ranges stay instant.
- Block variant creation with a modal error if any selected value or pending
input is invalid (out of range, off-increment, or not a number), so garbage
like '00A' can't reach creation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Marking an attribute numeric hides the Item Attribute Values grid but leaves
its rows in the doc, whose mandatory Attribute Value / Abbreviation block the
save client-side before the server can clear them. Clear the table on the
client too so the save goes through.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wraps per-(item, warehouse) reposting in repost_future_sle with a session-level advisory lock in front of the existing for-update row locks -- an outer gate that turns lock-order deadlocks into an orderly wait. Postgres/MariaDB only; nullcontext elsewhere. Row locks still enforce correctness. Requires frappe advisory_lock.
Reimplements get_ancestor_boms, BOM.traverse_tree, Task.check_recursion and the BOM Explorer report as single recursive-CTE queries (frappe.qb recursive=True), replacing query-per-node walks (N->1). Task cycle detection now catches cycles at any depth (was capped at 15 nodes). Requires the framework recursive-CTE support (frappe#40464).
Pick Lists transferred before this feature have transferred_qty = 0 and
their Stock Entry rows carry no pick_list_item link, so the new
is_fully_transferred check would never fire and, with the old
duplicate-entry guard removed, they could be transferred again. Set
transferred_qty = picked_qty for non-Delivery submitted pick lists that
already have a linked Stock Entry so they stay completed and locked.
work order status was decided using a stale transferred-qty value,
computed before the current stock entry's transfer got recomputed.
this left work orders stuck at "not started" for pick-list-driven
transfers, since those entries never set fg_completed_qty and their
transferred qty can only be known from actual item-level transfers.
an earlier attempt fixed this by setting fg_completed_qty from the pick
list's for_qty, but that broke two things tied to fg_completed_qty
being zero: the excess-transfer guard, and the partial-transfer
fraction logic used to avoid marking a work order as fully supplied too
early.
recompute the transferred qty first, then decide status from the fresh
value. revert the fg_completed_qty change since it's no longer needed.
Extend the boundary rule to callers: non-decorated code that built or
annotated with ItemDetailsCtx now uses frappe._dict directly, and drops
the now-unused import. asset_capitalization keeps ItemDetailsCtx for its
own normalize_ctx_input-decorated functions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ItemDetailsCtx signals the normalize_ctx_input boundary, so keep it only
on the decorator and the ctx param of decorated functions. Every other
annotation/constructor in non-decorated code becomes plain frappe._dict.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ensure the recursion guard only applies to the nested save() and is cleared
afterwards, so a later save() on the same doc instance still creates periods.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
get_item_price is an internal, non-decorated helper: the "| dict" and
"pctx = frappe._dict(pctx)" were load-bearing (callers may pass a plain
dict; the body does attribute access). Restore both. Also restore the
"| dict" on set_valuation_rate/update_party_blanket_order out params
(these are not normalize_ctx_input-decorated).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
on_update() called self.save(), which re-enters on_update() via
run_post_save_methods(), recursing indefinitely when make_all_scorecards()
keeps returning newly created periods. Guard the re-save with an in_rescore
flag so the nested on_update() short-circuits, while still running the full
validate() once to refresh score and standings.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Python 3.14 (PEP 649/749) replaced "__annotations__" with "__annotate__"
in functools.WRAPPER_ASSIGNMENTS. normalize_ctx_input excluded only
"__annotations__" when wrapping, so functools.wraps copied the wrapped
function's __annotate__ and the wrapper's permissive ctx annotation
(_dict | Document | dict | str) was overwritten by the narrow
ItemDetailsCtx | str. Now that Frappe casts whitelisted args via
typing_validations, a dict ctx failed the isinstance-only frappe._dict
check and raised FrappeTypeError. Exclude "__annotate__" too.
Cleanup while here:
- Merge the three identical frappe._dict aliases (ItemDetails,
ItemDetailsCtx, ItemPriceCtx) into ItemDetailsCtx.
- Drop the now-redundant "| str" from decorated signatures; the
decorator's wrapper union is what typing_validations enforces.
- Decorate get_batch_based_item_price with normalize_ctx_input instead
of a manual parse_json, renaming its arg pctx -> ctx (JS caller
updated) so a dict/string payload is normalized to frappe._dict.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Creating a Stock Entry from a Pick List blocked any further entry
(stock_entry_exists) and flipped the pick list to Completed as soon as
one entry existed, so picked stock could not be transferred in parts.
Track transferred_qty per Pick List Item (summed from submitted Stock
Entry rows via a new pick_list_item link, mirroring delivered_qty), add
a Partially Transferred status, and map each new Stock Entry from the
remaining qty so transfers can continue until fully transferred.
Both server-test workflows now name the test job "Python Unit Tests" so the
check appears under the same name regardless of engine.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match the framework rename of the standard report entry point in the
trial balance, P&L, balance sheet, and general ledger reports.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror frappe/erpnext#56655 for the Postgres CI. Run the
bootstrap_test_data module in the setup job while Postgres is still up, so
the BootStrapTestData records are baked into the PGDATA artifact every shard
hydrates from — the shards start on already-warmed data instead of each
building it.
Unlike the MariaDB step, no `su -m` wrapper: the Postgres CI is
GitHub-hosted ubuntu-latest running as the runner user directly, matching
its own "Run Tests" step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirror the config.json guidance in POSTGRES_COMPATIBILITY.md: when scoping a rollback, keep the function's
success/None return contract -- don't return the doc that was just rolled back. (greptile #56688)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirror the POSTGRES_COMPATIBILITY.md rule into the greptile instructions: prefer a scoped savepoint over a
full frappe.db.rollback() when recovering a poisoned txn; 'owns the txn' is not safe in a loop handler; and
keep the success/None return contract when scoping.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Recovering a poisoned Postgres txn with a full frappe.db.rollback() discards rows the handler already
created before the failure -- which MariaDB keeps (no statement-abort) -- so it's a silent MariaDB
regression. 'Owns the txn' does not make a full rollback safe in a loop handler. Document the safe cases
(re-raise / single op / atomic batch) and the per-iteration/per-record savepoint alternative.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
setUp creates/receives/delivers the item as '_Test Item with Serial No' (lowercase w) but the report
filter used '_Test Item With Serial No' (capital W). MariaDB's case-insensitive collation resolved it,
but Postgres (case-sensitive) matched no item, so the report's 'if items:' guard dropped the item
filter and returned serial-no rows for every item in the window -- an order-dependent, flaky count on
Postgres. Align the filter to the created item's case (a no-op on MariaDB).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Preserve the pre-existing contract: create_customer returned None when contact/address linking failed.
The savepoint fix kept the Customer (good) but started returning its name in that case, so a CRM caller
treating a non-None return as full success could skip its retry/error handling. Return None on a linking
failure while still keeping the Customer. (greptile #56683)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
create_customer wrapped customer.insert() + create_contacts() + create_address() in one try whose except
did a full frappe.db.rollback(), so a failure while linking contacts/address discarded the Customer just
created (MariaDB kept it pre-migration). Split the try: the customer insert keeps its full rollback (safe
-- nothing precedes it), and contact/address linking runs under a savepoint so its failure rolls back only
the links, preserving the Customer and healing the Postgres txn.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
new_bank_transaction inserts+submits Bank Transactions in a loop within one transaction. On a failed
insert/submit, Postgres poisons the transaction so the except's log_error dies with InFailedSqlTransaction;
MariaDB keeps the Bank Transactions synced before the failure. A full rollback would discard those on
MariaDB too, so wrap each iteration in a savepoint + rollback(save_point=) and re-raise -- preserves
MariaDB's partial-sync behaviour and heals the Postgres txn. The sibling handlers add_institution /
add_bank_accounts were already fixed; this closes the third.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The daily scheduler loops creating next-year Fiscal Years (autoname=field:year). A duplicate-year
INSERT aborts the statement; on Postgres that poisons the whole transaction, so the next iteration's
get_doc/insert dies with InFailedSqlTransaction. MariaDB statement-rolls-back and continues. Wrap each
iteration in a savepoint + rollback(save_point=) in the DuplicateEntryError branch -- a strict no-op on
MariaDB (same INSERT, same skip), recovers the txn on Postgres.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The contact lookup orders only by is_primary_contact desc then takes contacts[0]; contacts commonly
tie (the no-primary case), so MariaDB and Postgres could pick a different contact. Add a parent
(contact name) tiebreaker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Pick List Item get_value orders only by qty desc; a pick list can hold multiple rows for the same
SO item split across warehouses/batches/serials that tie on qty, so MariaDB and Postgres could stamp a
different warehouse/batch/serial onto the packed item. Add a name tiebreaker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
get_open_payment_requests_for_references orders by Coalesce(transaction_date, creation); when
transaction_date is set the coalesce never falls back to creation, so PRs sharing a transaction_date
have no tiebreaker and MariaDB/Postgres can allocate a different PR first. Append creation, name keys.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
get_item_price ORDER BYs valid_from/batch_no/uom/party then LIMIT 1 with no unique key. Two Item
Price rows tied on all of those but differing price_list_rate would be picked arbitrarily -- MariaDB
and Postgres can return a different rate. Append a name tiebreaker; for exact ties MariaDB's pick was
already undefined, so its output is preserved.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The QI-by-purpose check required an inspection on every outgoing
(s_warehouse) row for any purpose that was not incoming. Material
Consumption for Manufacture rows are source-only and the Work Order
mapper copies inspection_required from the BOM, so this silently
blocked submission. An inspection_required BOM inspects the finished
good, not each consumed raw material.
Replace the "anything not incoming" fallthrough with an explicit
QI_OUTGOING_PURPOSES allow-list (mirrored in transaction.js) so a new
purpose cannot silently start requiring a QI. Consumption and Return
Raw Material to Customer now need no QI; Issue, Transfer, Transfer for
Manufacture, Send to Subcontractor, Subcontracting Delivery and
Disassemble keep their outgoing checks. Scope item_query to the same
set and add a regression test.
The closing paragraph named only the §2/§3 semantic divergences as static-checker-invisible;
§6 (refactor/conversion row-set changes) is equally invisible and belongs there too. (greptile nit.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirror the two new POSTGRES_COMPATIBILITY.md rules into the greptile instructions so the bot flags
them on changed queries: (1) adding an ORDER BY column to a SELECT DISTINCT grows the distinct key
and the MariaDB row count unless it is functionally dependent; (2) a refactor / raw-frappe.db.sql->qb
conversion can silently change the WHERE/row set on both engines -- review the predicate, not just
the query shape.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two review lessons from the post-merge net-diff/whole-repo re-audit of the SQL-dialect classes:
- Section 3 (row-count trap) now covers SELECT DISTINCT too: adding the ORDER BY column to the
select to satisfy Postgres grows the DISTINCT key and changes the MariaDB row count when the
column is not single-valued per distinct row -- sort in Python instead.
- New section 6: a 'refactor' / raw-SQL->qb conversion is not automatically 1:1. Diff the
WHERE/predicate and the resulting row set, not just the SELECT shape -- a conversion that widens
a filter (e.g. posting_datetime > X gaining an OR (== X AND creation > ...) branch under a
sql->qb refactor) changes the rows touched on both engines and hides under a refactor label.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(stock): value batch/serial return from ledger when original receipt has no bundle
* test(stock): add test to validate the valuation of serial/batch for return when original receipt has no bundle
create_address is a helper called by create_prospect/create_customer AFTER they insert the Prospect/Customer. Its full frappe.db.rollback() on an address-save failure rolled back the caller's just-inserted parent doc, then swallowed the exception, so the caller returned a Prospect/Customer name that no longer existed. Scope the rollback to savepoint('crm_create_address') so only the address work is undone; the parent doc survives and the failed address is just logged.
link_existing_conversations is the Contact after_insert hook; a full frappe.db.rollback() on a failed call_log.save() would discard the triggering Contact insert itself (and, in test mode, the whole unit of work). Savepoint the hook's DB work and roll back only to it.
from_detailed_data inserts tax templates/accounts before update_regional_tax_settings in the same transaction; a full frappe.db.rollback() on regional-setup failure discarded those templates while the wizard continued. Take a savepoint before the regional call and roll back only to it.
After rollback(save_point=reorder_mr) discards the just-inserted Material Request, mr.log_error() left a dangling Error Log reference. Use frappe.log_error(title=...).
The savepoint rollback erases the just-inserted Bank Transaction row, so bank_transaction.log_error() created an Error Log pointing at a row that no longer exists. Use frappe.log_error(title=...) with no doc reference.
send_mail (called per campaign schedule in a loop) inserts a Communication via make(); on failure the except calls frappe.log_error with no rollback, raising InFailedSqlTransaction on Postgres and poisoning subsequent sends. Savepoint before make() + rollback(save_point=) before log_error. No-op on MariaDB.
make_depreciation_entry posts a Journal Entry per schedule row in a loop; the except only stored the error, so the next row's je.save()/submit() ran on the Postgres-poisoned txn (InFailedSqlTransaction). Savepoint per iteration + rollback(save_point=) before storing the error; the final raise of the collected error is unchanged. No-op on MariaDB.
trigger_invoice_update_for_subscriptions loops invoices calling refresh_subscription_status (db_set/save); on failure the except calls frappe.log_error with no rollback, raising InFailedSqlTransaction on Postgres, and the next invoice runs in the poisoned txn. Savepoint per iteration + rollback(save_point=) before log_error. No-op on MariaDB.
start_merge merges accounts in a loop; on failure it only rolled back when not in_test, so in tests a failed merge_account left the Postgres txn poisoned and the except log_error + the finally db_set(status) raised InFailedSqlTransaction. Wrap each row in savepoint('ledger_merge_row') and rollback to it unconditionally before log_error - this recovers the txn in both paths without the full rollback discarding the rest of the test transaction. Production still commits per successful merge, so the per-iteration savepoint rollback is equivalent to the prior full rollback. No-op on MariaDB.
create_prospect/create_address/create_customer insert docs and on failure call frappe.log_error with no rollback; on Postgres (untrusted external CRM webhook input) a failed insert poisons the txn so log_error raises InFailedSqlTransaction. Full frappe.db.rollback() before each log_error. No-op on MariaDB.
book_deferred_entries' make_gl_entries failure path: the else branch already rolls back before log_error, but the frappe.in_test branch ran doc.log_error then re-raised with no rollback -> on Postgres log_error hits InFailedSqlTransaction and masks the original error. Rollback before log_error in the in_test branch too. No-op on MariaDB.
create_production_plan_bom (background job) save+submits BOMs in a loop; on failure the except runs self.db_set(status=Failed, error_log) with no rollback, raising InFailedSqlTransaction on Postgres so status is never set. Full frappe.db.rollback() at the top of the except. No-op on MariaDB.
prepare_closing_stock_balance (background job) saves Stock Closing Balance rows + db_set status; on failure the except runs db_set('Failed')+log_error with no rollback, raising InFailedSqlTransaction on Postgres so the doc is never marked Failed and the job dies. Full frappe.db.rollback() before the handler's db_set. No-op on MariaDB.
Regional tax-template setup writes docs; on failure the except calls frappe.log_error with no rollback -> InFailedSqlTransaction on Postgres. Full rollback before log_error. No-op on MariaDB.
Regional fixture setup writes docs; on failure the except calls frappe.log_error before frappe.throw with no rollback -> InFailedSqlTransaction on Postgres. Full rollback before log_error. No-op on MariaDB.
The hook saves Call Logs in a loop; on failure the except calls frappe.log_error (INSERT) with no rollback, raising InFailedSqlTransaction on Postgres (it runs on every Contact create/update). Full frappe.db.rollback() before log_error. No-op on MariaDB.
add_bank_accounts hardened only the INSERT branch with savepoint('plaid_bank_account'); the parallel else/UPDATE branch ran log_error+throw after a failed existing_account.save() with no rollback -> InFailedSqlTransaction on Postgres (masking the friendly throw). Mirror the insert branch with savepoint('plaid_update_account')+rollback. Also add_institution's except log_error after a failed bank.insert() now rolls back first. No-op on MariaDB.
create_material_request loops companies inserting+submitting a Material Request; the except calls mr.log_error (INSERT) with no rollback, raising InFailedSqlTransaction on Postgres in the scheduled reorder job, and the next company runs in the poisoned txn. Savepoint per iteration + rollback(save_point=) before log_error. No-op on MariaDB.
create_bank_entries loops rows inserting+submitting a Bank Transaction; on failure the except calls bank_transaction.log_error (INSERT) with no rollback, raising InFailedSqlTransaction on Postgres, and the next row runs in the poisoned txn. Savepoint per row + rollback(save_point=) before log_error. No-op on MariaDB.
make_default_records inserted Scorecard Variable/Standing rows in a loop and swallowed DuplicateEntryError (frappe.NameError). On Postgres the failed insert poisons the txn so the next iteration's insert raises InFailedSqlTransaction. insert(ignore_if_duplicate=True) emits ON CONFLICT DO NOTHING, never poisoning the txn. No-op on MariaDB.
Same shape: the rows loop submits a Repost Item Valuation; a caught DuplicateEntryError poisons the Postgres txn so the next iteration's submit raises InFailedSqlTransaction. Savepoint + rollback(save_point=) before continue. No-op on MariaDB.
The item/warehouse loop submits a Repost Item Valuation; a DuplicateEntryError poisons the Postgres transaction, so the next iteration's .submit() raises InFailedSqlTransaction. MariaDB continues. Savepoint per iteration + rollback(save_point=) on the caught duplicate (mirrors repost_item_valuation:782). No-op on MariaDB.
get_exchange_rate orders Currency Exchange by 'date desc' LIMIT 1 with no unique tiebreaker. Currency Exchange autoname {date}-{from}-{to}-{purpose} allows multiple same-date rows (different purpose) for one currency pair; on the no-purpose-filter path all match, so MariaDB and Postgres can return a different exchange_rate for the same inputs. Add 'name desc' so both engines pick the same row. MariaDB row count unchanged.
get_rate_for_return builds Abs(stock_value_difference / actual_qty) for Sales/Delivery returns and passes it to get_value with no actual_qty filter. A matched Stock Ledger Entry with actual_qty=0 (a zero-qty repost / serial-batch row) makes Postgres raise 'division by zero' while MariaDB returns NULL. Wrap the divisor in NullIf(actual_qty, 0) so both engines return NULL. MariaDB output unchanged. Sibling of the already-fixed /actual_qty sites in stock_ledger.py and incorrect_serial_no_valuation.py.
create_purchase_invoice caught its own failure and then ran frappe.db.set_value + log_error in the SAME transaction. On Postgres a failed insert/save aborts the whole transaction, so the error-marking died with InFailedSqlTransaction and the failure cascaded through prepare_data_for_import's per-file loop, killing the entire import; MariaDB recovers per-statement and continues.
Let create_purchase_invoice raise, and wrap each call in prepare_data_for_import in frappe.db.savepoint + rollback(save_point=...). On failure the savepoint rollback un-poisons the transaction, the error is logged, and the per-file status is set to Error and committed (self.db_set(commit=True), matching the existing process_file_data status commit) so an interrupted import durably reflects Error instead of staying at the already-committed Processing File Data; the loop then continues to the next file. The savepoint is taken after create_supplier/create_address so those are preserved exactly as before.
Behaviour change (MariaDB): a failed invoice's partially-created draft Purchase Invoice is now rolled back on BOTH engines instead of being left as an orphan draft on MariaDB. Deliberate and more correct - a failed import should not leave a partial invoice; release-note worthy.
Greptile flagged that `gh release download` with `github.token` could be
rejected for fork pull requests (token scoped to the fork, asset in
frappe/erpnext). The release is public and published, so the asset is
downloadable anonymously from objects.githubusercontent.com — drop the token
and curl the public URL directly. Removes the cross-repo token dependency and
keeps fork PRs working. Cloudflare is still bypassed since GitHub serves the
asset, not frappe.io.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The "Stop DB and stage datadir" step swallowed a failed `pg_ctl -m fast -w
stop` with `|| true`, then moved and tarred the PGDATA regardless. A stop
that times out or errors would bake a still-running, crash-inconsistent
cluster into the artifact every test shard consumes — and with
full_page_writes off, crash recovery can't repair torn pages. Drop the
`|| true` so a failed stop fails the job, mirroring the MariaDB sister's
"don't bake a dirty datadir" guard.
Also drop the redundant `ALTER SYSTEM SET fsync/synchronous_commit/
full_page_writes = off` block from install.sh. Its comment claimed the
postgres workflow "runs a service-container DB and never calls start-db.sh",
but it does call start-db.sh, which already applies those flags via `-o` on
every postgres start (setup job and each shard). The block was a no-op and
its justification was factually wrong.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Patch Test job intermittently failed on the "Download erpnext v14 backup"
step with HTTP 403 Forbidden: frappe.io sits behind Cloudflare, and wget's
default User-Agent gets flagged by bot protection on cache misses. This caused
random failures across PRs that only a re-run would clear.
Pull the fixed baseline from the v14-baseline GitHub release using the built-in
token instead. Release assets are served from GitHub's CDN and authenticated
from the runner, so no rate-limit roulette.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
create_routing inserts a Routing and, on DuplicateEntryError, re-fetches and updates. On Postgres the failed insert aborts the transaction so the get_doc/save in the except raises InFailedSqlTransaction; MariaDB recovers. Savepoint + rollback before the fallback path.
make_reposting_for_accounting_ledgers submits a new Repost Item Valuation per voucher in a loop under except Exception. On Postgres a failed submit aborts the transaction so the next iteration's DB work dies with InFailedSqlTransaction; MariaDB continues. Savepoint per iteration, roll back on failure. No-op on MariaDB.
Same submit()/add_comment-in-except shape as the PO->SCO mapper: on Postgres a failed submit aborts the transaction so the follow-on Comment insert raises InFailedSqlTransaction; MariaDB continues. Savepoint + rollback before add_comment. No-op on MariaDB.
target_doc.submit() is wrapped in except Exception whose handler calls add_comment (a Comment insert). On Postgres a failed submit poisons the transaction so the add_comment insert raises InFailedSqlTransaction; MariaDB logs the comment. Savepoint + rollback before add_comment. No-op on MariaDB.
get_last_sales_amt ordered by the sales date DESC only; same-date documents made the reported Last Order Amount engine-dependent. Add name DESC tiebreaker.
get_last_purchase_rate ordered by posting_date DESC only; same-date Purchase Invoices yielded an undefined last_purchase_rate that diverged between MariaDB and Postgres. Add purchase_invoice.name DESC tiebreaker.
get_latest_location_and_custodian ordered by transaction_date DESC only; equal-dated movements left the current location/custodian engine-dependent. Add asm.name DESC tiebreaker so both engines pick the same movement.
calculate_exchange_rate_using_last_gle ordered the latest-GLE lookups by posting_date DESC only. With multiple GL Entries on the latest posting_date the picked row was undefined, so MariaDB and Postgres could choose different vouchers and return a different last_exchange_rate (and revaluation gain/loss). Add gl.name DESC as a tiebreaker so both engines pick the same row; MariaDB row count unchanged.
The "Is Fully Depreciated" field was hidden on the Asset form (hidden: 1),
so it could never be set for manually entered existing assets.
Make it visible based on context:
- Existing Asset with Calculate Depreciation off -> visible and editable
- Calculate Depreciation on -> visible but read-only and forced unchecked
(it is only meaningful for manually entered assets)
The unchecked value is enforced in the form script (immediate feedback on
toggle and on load) and in server-side validate() so it can never be saved
as checked while depreciation is being calculated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deduplicate the identical confirmation dialog used by Item and Stock
Settings into erpnext.utils.confirm_negative_stock, and collapse the
message into a single translatable string.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fetch QI items by warehouse direction per inspection_type, require QI on
the correct rows per stock entry purpose (finished good for Manufacture,
inward goods for Receipt/Repack, outgoing rows for issue/transfer), and
show the QI field only on those rows.
- pay multiple purchase invoices with a single Payment Entry
- unallocated (advance) amount when a supplier payment is overpaid
- allocating more than a purchase invoice's outstanding amount is rejected
constcontent=_("Below is a list of all accounting entries posted against the bank account {0} between {1} and {2}.",[`<strong>${bankAccount?.account}</strong>`,`<strong>${formattedFromDate}</strong>`,`<strong>${formattedToDate}</strong>`])
return<divclassName="space-y-4 py-2">
<div>
<ParagraphclassName="text-sm">
<spandangerouslySetInnerHTML={{
__html: _("Below is a list of all accounting entries posted against the bank account {0} between {1} and {2}.",[`<strong>${bankAccount?.account}</strong>`,`<strong>${formattedFromDate}</strong>`,`<strong>${formattedToDate}</strong>`])
constcontent=_("Below is a list of all entries posted against the bank account {0} which have not been cleared till {1}.",[`<strong>${bankAccount?.account}</strong>`,`<strong>${formatDate(dates.toDate)}</strong>`])
return<divclassName="space-y-4 py-2">
<div>
<ParagraphclassName="text-sm">
<spandangerouslySetInnerHTML={{
__html: _("Below is a list of all entries posted against the bank account {0} which have not been cleared till {1}.",[`<strong>${bankAccount?.account}</strong>`,`<strong>${formatDate(dates.toDate)}</strong>`])
constcontent=_("Below is a list of all bank transactions imported in the system for the bank account {0} between {1} and {2}.",[`<strong>${bankAccount?.account_name}</strong>`,`<strong>${formattedFromDate}</strong>`,`<strong>${formattedToDate}</strong>`])
__html: _("Below is a list of all bank transactions imported in the system for the bank account {0} between {1} and {2}.",[`<strong>${bankAccount?.account_name}</strong>`,`<strong>${formattedFromDate}</strong>`,`<strong>${formattedToDate}</strong>`])
constcontent=_("This report shows all entries in the system where the <strong>clearance date is before the posting date</strong> which is incorrect.")
constentriesContent=_("Entries below have a posting date after {0} but the clearance date is before {1}.",[`<strong>${formattedToDate}</strong>`,`<strong>${formattedToDate}</strong>`])
return<divclassName="space-y-4 py-2">
<div>
<ParagraphclassName="text-sm">
<spandangerouslySetInnerHTML={{
__html: _("This report shows all entries in the system where the <strong>clearance date is before the posting date</strong> which is incorrect.")
}}/>
<spanclassName="text-p-sm">
<MarkdownRenderercontent={content}/>
<br/>
{data&&data.message.result.length>0&&<span>
<spandangerouslySetInnerHTML={{
__html: _("Entries below have a posting date after {0} but the clearance date is before {1}.",[`<strong>${formattedToDate}</strong>`,`<strong>${formattedToDate}</strong>`])
}}/>
<MarkdownRenderercontent={entriesContent}/>
<br/>
{_("You can reset the clearing dates of these entries here.")}
"label":"Role Allowed to Bypass Over Billing Restriction",
"options":"Role"
},
{
"fieldname":"period_closing_settings_section",
"fieldtype":"Section Break"
@@ -757,6 +776,18 @@
"description":"Changing the account in any transaction of the DocTypes listed below will trigger a repost. To prevent reposting, remove the relevant DocType from the list.",
"fieldname":"column_break_mfor",
"fieldtype":"Column Break"
},
{
"fieldname":"stock_expense_section",
"fieldtype":"Section Break",
"label":"Stock Expense Accounting"
},
{
"default":"0",
"description":"Books Purchase Expense and Expenses Added To Stock account pairs against stock value. On enabling this, the accounts become mandatory in Company or Item Defaults for Purchase Receipt, Purchase Invoice, Stock Entry, Stock Reconciliation and Landed Cost Voucher",
"The closing balance {0} of the Stock Asset accounts does not match the closing value {1} of the Stock Balance report as on {2}. Resolve the difference using the Stock Ledger Variance report before closing the period."
"Stock transactions were created or modified after the Stock Closing Entry {0} was generated. Regenerate it before submitting the Period Closing Voucher."
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
