[Closed][Subcontracting] Fix subcontracting order opening wrong purchase order after creation#8811
[Closed][Subcontracting] Fix subcontracting order opening wrong purchase order after creation#88116CRIPT wants to merge 15 commits into
Conversation
…he last one created
6CRIPT
changed the title
Copilot PR ReviewIteration 15 · Outcome: completed Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630 Findings by domainFindings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).
Totals: 0 knowledge-backed · 1 agent findings. Orchestrator pre-filter (13 file(s) excluded)
Findings produced by the Copilot CLI agent against BCQuality at |
There was a problem hiding this comment.
Pull request overview
This PR addresses an issue in the Subcontracting app where, after creating subcontracting purchase orders from production order routing lines, the UI could open the wrong purchase order (typically an existing one matching the same routing-line filters).
Changes:
- Track and mark newly created purchase order headers by diffing purchase lines before/after running “Carry Out Action Msg. - Req.”.
- Update the “show created purchase order(s)” flow to open the marked purchase order(s) instead of searching by routing/operation fields.
- Remove now-unneeded operation/routing-reference state plumbing from the routing page extension.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Apps/W1/Subcontracting/App/src/Purchase/SubcPurchaseOrderCreator.Codeunit.al | Adds logic to collect existing PO numbers, mark newly created PO headers, and use marked headers when showing created orders. |
| src/Apps/W1/Subcontracting/App/src/Manufacturing/SubcProdOrderRtng.PageExt.al | Removes operation/routing-reference setter calls since the creator codeunit now tracks created orders internally. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
Should we add a test? |
Agentic PR Review - Round 4Recommendation: Accept with SuggestionsWhat this PR doesThe round-4 commit only updates the two regression tests by removing the extra The production code did not change. The tests still exercise the affected navigation paths: the received-old-PO case opens the newly created PO, and the open-existing-PO case reopens the affected existing PO. Status of previous suggestions
New observations (commits since round 3)None. The only new commit adjusts the tests and does not introduce a new functional issue. Risk assessment and necessityRisk: The round-4 change is test-only, so it does not add product regression risk. The main fix remains narrow and tested for the old-received-PO delta case and the open-existing-PO update case. The remaining risk is still the existing-orders prompt, which can show broader purchase lines for the production order because it drops the routing filters before opening the list. Necessity: AB#639381 is a clear Bug with strong repro steps. The created-order navigation fix is needed because users can otherwise be sent to the old received PO and must find the new PO manually. The round-4 test change is reasonable because it keeps the regression tests aligned with the repro flow.
|
…tstanding Amount - SalesDocTotalsSalesEntryUI: disable Foundation app area to prevent auto invoice discount calculation - PurchDocTotalsSalesEntryUI: same fix for purchase doc totals tests - ERMSalesPurchaseDocument (CA): use Amount Including VAT for customer GL entry ACY calculation - SalesPurchaseReports (CA): use Outstanding Amount (includes tax) for report verification"
github-actions
Bot
added
the
needs-approval
…urchaseOrderCreator
…, and Outstanding Amount" This reverts commit cd26c51.
github-actions
Bot
removed
the
needs-approval
Agentic PR Review - Round 5Recommendation: Accept with SuggestionsWhat this PR doesThe round-5 diff has one net AL product change in This is safe on that path. After Status of previous suggestions
New observations (commits since round 4)None. Commit Risk assessment and necessityRisk: The round-5 product change is very small and only affects the no-marked-header early-exit path in the created-purchase-order display flow. It does not change the successful path that opens the marked purchase order or list. The remaining risk is still the existing-orders prompt, which can show broader purchase lines for the production order because it drops the routing filters before opening the list. Necessity: AB#639381 is a Bug with clear repro steps and an AI repro score of 9/10. The main created-order navigation fix is needed because users can otherwise be sent to the old received PO and must find the new PO manually. The round-5 cleanup is reasonable because resetting
|
…cting-order-opens-old-purchase-order
6CRIPT
force-pushed
the
bugs/639381-subcontracting-order-opens-old-purchase-order
branch
from
18b61c6 to
785995f
Compare
6CRIPT
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| PurchaseLine.SetRange("Routing Reference No.", ProdOrderRoutingLine."Routing Reference No."); | ||
| PurchaseLine.SetRange("Operation No.", ProdOrderRoutingLine."Operation No."); | ||
| PurchaseLine.SetRange("Planning Flexibility", "Reservation Planning Flexibility"::Unlimited); | ||
| PurchaseLine.SetRange("Quantity Received", 0); |
There was a problem hiding this comment.
In MarkNewlyCreatedSubcPurchaseOrder (SubcPurchaseOrderCreator.Codeunit.al), when Carry Out updates an existing open purchase order instead of creating a new one (MarkedPurchaseOrderNos.Count() = 0 after the first pass), the fallback loop marks every purchase line still matching FilterSubcPurchaseLineForRoutingLine as 'affected', not just the one order Carry Out actually touched.
If more than one open, unreceived purchase order already matches that exact routing line (same Prod. Order No./Line No./Routing No./Operation No., Planning Flexibility = Unlimited, Quantity Received = 0) -- e.g. a user previously split the order manually -- all of them get Marked, and ShowCreatedPurchaseOrder will subsequently open/list orders the current Carry Out run did not modify. Recommend narrowing the fallback to the specific document Carry Out changed (for example by diffing quantities/dates against a snapshot taken in CollectExistingSubcPurchaseOrderNos) rather than marking every remaining filter match.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
CLOSED BECAUSE COPILOT ADDED NOISE WITHOUT PERMISSION.
CREATING A NEW CLEAN PR: #9396
Fixes AB#639381
Problem
When a subcontracting operation already had a received purchase order and the production order quantity was increased, clicking Create Subcontracting Order again created a new PO for the delta quantity but opened the old, already-received PO instead of the newly created one. The user had to manually navigate to find the correct order.
Root cause
ShowCreatedPurchaseOrder re-derived which PO to open by filtering purchase lines (Prod. Order No. + Operation No. + Routing Reference No.). That filter matched both the old and the new lines, so FindFirst returned the lowest Document No. (the old one). The code never tracked which document was actually created during the current run.
Fix
We now capture the purchase order(s) actually created/affected in the current invocation: we snapshot the existing subcontracting POs before Carry Out and Mark the newly created (or updated) header afterwards, then open only the marked set. Filtering was aligned with PurchLineExists (open, unreceived lines), the confirmation count/branching is derived from the marked headers, and a fallback keeps working when Carry Out updates an existing open PO instead of creating a new one. Added two integration tests covering the delta-PO and updated-open-PO scenarios.