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>
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>
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.
