frm.toggle_display("update_stock", ...) unconditionally forced the
field visible based only on has_subcontracted, overwriting whatever
Customize Form had set on every refresh. OR it with the field's
original (property-setter-driven) hidden value instead.
Backport of #57818.
* feat: validate stock value and stock closing entry before period closing
(cherry picked from commit 20450bd4ec)
* fix: do not accept scoped stock closing entries as period closing prerequisite
(cherry picked from commit 359a347be2)
* feat: seed batch valuation from stock closing balance and freeze closed-period stock
(cherry picked from commit 49a127d59c)
* fix(subscription): don't reactivate a cancelled subscription
Backport of #57774 to version-16-hotfix.
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
* test: fix flaky test_update_bom_cost_in_all_boms via valuation reset
Backport of #56796 to version-16-hotfix.
reset_item_valuation_rate() only reconciled warehouses where the item
currently has positive stock (actual_qty > 0). get_valuation_rate()
averages Sum(stock_value)/Sum(actual_qty) across all of an item's
bins, so a negative balance left over in another warehouse by a prior
test can cancel out the reset qty and collapse the average to 0,
failing the assertion with 0.0 != 10.0.
This branch never got #56796 (it predates the frappe.get_all
refactor of this helper and still uses raw SQL), so applying the same
fix here: reconcile every warehouse with a non-zero balance, not just
positive ones.
* fix(subscription): don't let period rollover defeat cancel_at_period_end
process() can advance current_invoice_end to the next period (via
update_subscription_period(), when generating the current period's
invoice) before the cancel_at_period_end check further down runs. For
a subscription with no end_date, that check now compared posting_date
against the already-rolled-forward current_invoice_end, which is
always in the future, so cancel_at_period_end was silently never
honored.
Snapshot current_invoice_end before any rollover and use that in the
check instead, so it still targets the period that just ended.
Found via review on the version-15-hotfix backport (#57780).
---------
Co-authored-by: test <test@test.com>
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: rohitwaghchaure <rohitw1991@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
(cherry picked from commit 097ce0f348)
Backport of five fixes merged to develop, adapted to this branch, where
the field is still named `type` and the stock entry rate logic has not
been split out of set_basic_rate.
- A secondary row with no BOM link is costed out of the finished good,
as legacy scrap was. Finished goods are rated last so a single
validate pass sees the secondary rows' amounts. (#57732)
- Repack no longer flags secondary rows as finished goods, so each side
takes the share the BOM declares instead of the scrap absorbing the
finished good's percentage. (#57735)
- A BOM allocation of 0% means the row carries no cost, rather than
falling through to the item's own valuation rate. (#57736)
- Secondary Item Type no longer waives a quality inspection on purposes
that do not produce secondary items. (#57737)
- The BOM allocation applies to the consumption entry's cost when the
raw material cost comes from one. (#57738)
Replaces the individual backports, which could not be cherry-picked
cleanly: every hunk needed rewriting against the pre-rename field and
the un-refactored rate logic.
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.
(cherry picked from commit 8d5326196e)
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.
(cherry picked from commit 25cd793617)
frappe.db.escape() wraps the value in quotes (e.g. "'Products'").
Callers pass the result into query-builder isin()/frappe.get_all
filters, which parameterize values themselves — so the pre-quoted
string never matches a real Item Group name, and POS shows no items
whenever a POS Profile restricts Item Groups.
Return raw names instead, matching develop.
fix(banking): fetch company list from DB instead of boot (#57731)
* fix(banking): fetch company list from DB instead of boot
* fix: show error banner for company list fail fetch
(cherry picked from commit abc3da6b97)
Co-authored-by: Nikhil Kothari <nik.kothari22@live.com>
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.
(cherry picked from commit 732c884633)
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.
(cherry picked from commit a3e9d13da3)
Keep validate_warehouses() alongside the new
validate_over_delivery_receipt_allowance() call.
Drop test_blanket_order_over_order_aggregated_across_rows: it is develop-only
context the cherry-pick swallowed into the conflict, not part of #57725.
Revert the valuation_method literal to the three options this branch offers -
Standard Cost rode along from a regenerated develop type block.
test_sales_return_validates_against_original came in with the new file,
not with the change being backported. It covers a raw-SQL to query-builder
conversion that only exists on develop, and it imports
erpnext.stock.doctype.delivery_note.mapper, a module version-16-hotfix
does not have.
The backport left both import hunks unresolved, so the file did not compile.
version-16-hotfix keeps item_query unannotated and still imports cstr, so only
get_number_format_info goes, replaced by NumberFormat; typing.Any is not
carried over because nothing on this branch uses it.
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.
(cherry picked from commit 00d17ca5db)
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.
(cherry picked from commit 5b5f354090)
# Conflicts:
# erpnext/stock/doctype/quality_inspection/quality_inspection.py
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.
(cherry picked from commit b1f188146e)
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.
(cherry picked from commit 3752be809f)
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.
(cherry picked from commit e74c0a3cdb)
