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.
