-
Notifications
You must be signed in to change notification settings - Fork 22
Extensibility requests triage + implementation #762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
ac3c1ff
976a94b
c59108e
1b32570
9ef01ab
81322b3
2647111
d221dfd
b66d50f
1f70eab
30e759a
01e5350
38635b0
e2b5b08
bffe360
268ab28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
AleksandricMarko marked this conversation as resolved.
Outdated
|
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # Extensibility request: three extension points in table 39 "Purchase Line" | ||
|
|
||
| ## Why this change is needed | ||
|
|
||
| Table 39 "Purchase Line" runs several pieces of standard logic that an extension currently cannot | ||
| skip or replace. In three places there is no event that lets a subscriber apply a custom condition | ||
| and, when it is met, run its own logic instead of the standard call: | ||
|
|
||
| 1. In the `"Location Code"` field `OnValidate` trigger, `PlanPriceCalcByField` is always called for | ||
| items when the location changes; there is no way to suppress it conditionally. | ||
| 2. In `CopyFromItem`, `GetItemTranslation` is always called when the purchase header has a language | ||
| code; there is no way to skip it and run custom translation handling instead. | ||
| 3. In `CheckWMS`, `CheckLocationOnWMS` is always called when `CurrFieldNo <> 0`; there is no way to | ||
| suppress that warehouse check conditionally. | ||
|
|
||
| We need an extension point before each of these calls so subscribers can plug in custom handling in a | ||
| supported, upgrade-safe way. | ||
|
|
||
| ## Requested change | ||
|
|
||
| Add three new integration events to table 39 "Purchase Line", each raised **before** the | ||
| corresponding standard call and each using the `IsHandled` pattern so a subscriber that sets | ||
| `IsHandled := true` skips the standard call: | ||
|
|
||
| 1. Before `PlanPriceCalcByField` in the `"Location Code"` `OnValidate` trigger. Pass the | ||
| `Purchase Line` record, `IsHandled` (var), the current field number, and the `xPurchaseLine` | ||
| record. | ||
| 2. Before `GetItemTranslation` in `CopyFromItem`. Pass the `Purchase Line` record, the `Item` record, | ||
| and `IsHandled` (var). | ||
| 3. Before `CheckLocationOnWMS` in `CheckWMS`. Pass the `Purchase Line` record, the current field | ||
| number, and `IsHandled` (var). `CheckWMS` needs a local `IsHandled: Boolean` variable added. | ||
|
|
||
| Illustrative shape for one of the three (final event names and signatures must follow BC event | ||
| conventions): | ||
|
|
||
| ```al | ||
| local procedure CheckWMS() | ||
| var | ||
| IsHandled: Boolean; | ||
| begin | ||
| IsHandled := false; | ||
| // new integration event raised here, passing Purchase Line, CurrFieldNo, and IsHandled (var) | ||
| if not IsHandled then | ||
| if CurrFieldNo <> 0 then | ||
| CheckLocationOnWMS(); | ||
| // ... | ||
| end; | ||
| ``` | ||
|
|
||
| ## Scope | ||
|
|
||
| - Table 39 "Purchase Line" is kept as a separate copy in many country/region layers. All three | ||
| events must be added consistently to **every** layer that keeps a copy of this table: | ||
| - `App/Layers/W1/BaseApp/Purchases/Document/PurchaseLine.Table.al` | ||
| - and the same file in the APAC, BE, CH, DACH, ES, FI, GB, IT, NA, NL, NO, RU, and SE layers. | ||
| - That is 14 layer copies in total, so exactly 14 files change. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # Extensibility request: extension point in table 5409 "Prod. Order Routing Line" before ModifyCapNeedEntries | ||
|
|
||
| ## Why this change is needed | ||
|
|
||
| In table 5409 "Prod. Order Routing Line", the `"No."` field `OnValidate` trigger transfers work | ||
| center or machine center fields (via `WorkCenterTransferFields` / `MachineCtrTransferFields`) and then | ||
| immediately updates capacity need entries by calling `ModifyCapNeedEntries`. | ||
|
|
||
| An extension that needs to run custom logic **after** all the standard transfer logic has completed | ||
| but **before** capacity need entries are recalculated has no way to hook in: there is no event raised | ||
| at this exact point in the execution flow. In particular, a subscriber that needs to evaluate both the | ||
| current and previous values of the routing line (`Rec` and `xRec`) after the transfer, but before | ||
| `ModifyCapNeedEntries`, cannot do so today. | ||
|
|
||
| ## Requested change | ||
|
|
||
| Add a new integration event to table 5409 "Prod. Order Routing Line", raised in the `"No."` field | ||
| `OnValidate` trigger **immediately after** the `case Type of` block (that performs the work/machine | ||
| center field transfer) and **immediately before** the `ModifyCapNeedEntries` call. The event should | ||
| pass the current record and the previous record (`xRec`) so subscribers can compare them. | ||
|
|
||
| Illustrative shape (final event name and signature must follow BC event conventions): | ||
|
|
||
| ```al | ||
| case Type of | ||
| Type::"Work Center": | ||
| begin | ||
| WorkCenter.Get("No."); | ||
| WorkCenter.TestField(Blocked, false); | ||
| WorkCenterTransferFields(); | ||
| end; | ||
| Type::"Machine Center": | ||
| begin | ||
| MachineCenter.Get("No."); | ||
| MachineCenter.TestField(Blocked, false); | ||
| MachineCtrTransferFields(); | ||
| end; | ||
| end; | ||
| // new integration event raised here, passing Rec and xRec | ||
| ModifyCapNeedEntries(); | ||
| ``` | ||
|
|
||
| ## Scope | ||
|
|
||
| - Table 5409 "Prod. Order Routing Line" is kept as a separate copy in the W1 base layer and in the IT | ||
| layer. The change must be applied consistently to both copies: | ||
| - `App/Layers/W1/BaseApp/Manufacturing/Document/ProdOrderRoutingLine.Table.al` | ||
| - `App/Layers/IT/BaseApp/Manufacturing/Document/ProdOrderRoutingLine.Table.al` | ||
| - No other layers keep a copy of this table, so exactly two files change. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Extensibility request: make "Item Jnl.-Post Line".InsertPostValueEntryToGL externally callable | ||
|
|
||
| ## Why this change is needed | ||
|
|
||
| Codeunit 22 "Item Jnl.-Post Line" exposes `PostInventoryToGL` as a public procedure, and extensions | ||
| already build on it to post additional value entries to the general ledger. The related helper | ||
| `InsertPostValueEntryToGL(ValueEntry: Record "Value Entry")` posts a single value entry to the G/L, | ||
| but it is declared `local`, so it cannot be called from an extension. | ||
|
|
||
| An extension that posts additional variance (or other custom value entries) needs to reuse this exact | ||
| standard logic instead of duplicating it. There is currently no supported way to do that. | ||
|
|
||
| ## Requested change | ||
|
|
||
| Widen the accessibility of the `InsertPostValueEntryToGL` method in codeunit 22 "Item Jnl.-Post Line" | ||
| so it can be called from outside the codeunit: change it from a `local procedure` to a (public) | ||
| `procedure`. The body and signature stay exactly the same. | ||
|
|
||
| ```al | ||
| // before | ||
| local procedure InsertPostValueEntryToGL(ValueEntry: Record "Value Entry") | ||
|
|
||
| // after | ||
| procedure InsertPostValueEntryToGL(ValueEntry: Record "Value Entry") | ||
| ``` | ||
|
|
||
| ## Scope | ||
|
|
||
| - This codeunit is kept as a separate copy in several country/region layers. The accessibility change | ||
| must be applied consistently to **every** layer that has a copy, so the method is public everywhere: | ||
| - `App/Layers/W1/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al` | ||
| - `App/Layers/APAC/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al` | ||
| - `App/Layers/CH/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al` | ||
| - `App/Layers/ES/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al` | ||
| - `App/Layers/IT/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al` | ||
| - `App/Layers/RU/BaseApp/Inventory/Posting/ItemJnlPostLine.Codeunit.al` | ||
| - No other layers keep a copy of this codeunit, so only these six files change. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # Extensibility request: extension point in "Job Archive Management".AutoArchiveJob | ||
|
|
||
| ## Why this change is needed | ||
|
|
||
| `Job Archive Management.AutoArchiveJob` decides how a job is archived based on | ||
| `Jobs Setup."Archive Jobs"`. Today it only handles the `Always` and `Question` options; any other | ||
| value silently does nothing, so there is no way for an extension to plug in custom archive handling | ||
| (for example, a partner wants a "Request Page" style flow that collects extra input from the user | ||
| during archiving). | ||
|
|
||
| We need an extension point so subscribers can react when neither of the standard options applies. | ||
|
|
||
| ## Requested change | ||
|
|
||
| Add an integration event in the `else` branch of the `case Jobs Setup."Archive Jobs"` statement in | ||
| `AutoArchiveJob`, so extensions can handle additional archive modes. The event should pass the `Job` | ||
| and `Jobs Setup` records to subscribers. | ||
|
|
||
| Illustrative shape of the request (final event name and signature must follow BC event conventions): | ||
|
|
||
| ```al | ||
| procedure AutoArchiveJob(var Job: Record Job) | ||
| var | ||
| JobSetup: Record "Jobs Setup"; | ||
| begin | ||
| JobSetup.Get(); | ||
| case JobSetup."Archive Jobs" of | ||
| JobSetup."Archive Jobs"::Always: | ||
| StoreJob(Job, false); | ||
| JobSetup."Archive Jobs"::Question: | ||
| ArchiveJob(Job); | ||
| else | ||
| // new integration event raised here, passing Job and Jobs Setup | ||
| end; | ||
| end; | ||
| ``` | ||
|
|
||
| ## Scope | ||
|
|
||
| - File: `App/Layers/W1/BaseApp/Projects/Project/Archive/JobArchiveManagement.Codeunit.al` | ||
| - This codeunit exists only in the W1 base layer, so the change lives in W1 alone (no country/region | ||
| layer counterparts to propagate to). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # Extensibility request: extension point in codeunit 5980 "Service-Post" before posted header creation | ||
|
|
||
| ## Why this change is needed | ||
|
|
||
| The extensibility of codeunit 5980 "Service-Post" is limited compared to codeunit 80 "Sales-Post". | ||
| In "Sales-Post", an integration event lets partners override the standard logic that decides whether | ||
| a posted invoice or a posted credit memo should be created, using the `IsHandled` pattern. | ||
|
|
||
| In "Service-Post" the equivalent decision is hardcoded: when a document is invoiced, the code decides | ||
| between preparing a posted service invoice header and a posted service credit memo header purely from | ||
| `Service Header."Document Type"`. Because there is no event before this logic, extensions cannot | ||
| support custom Service document types that need a different posting flow or a different posted | ||
| document. | ||
|
|
||
| We need an extension point so partners can replace this decision in a supported, upgrade-safe way, | ||
| bringing Service-Post in line with Sales-Post. | ||
|
|
||
| ## Requested change | ||
|
|
||
| Add a new integration event in codeunit 5980 "Service-Post", raised **immediately before** the | ||
| standard logic that determines whether a posted service invoice or posted service credit memo is | ||
| created (the `if Invoice then ...` block that calls `PrepareInvoiceHeader` / `PrepareCrMemoHeader`). | ||
|
|
||
| Use the `IsHandled` pattern, matching the existing behavior in "Sales-Post": | ||
|
|
||
| - Declare an `IsHandled: Boolean` local variable and initialize it to `false`. | ||
| - Raise the new event before the standard block, passing enough context for a subscriber to take | ||
| over: the `Service Header`, the `IsHandled` flag (by reference), and the posted invoice and credit | ||
| memo numbers (by reference). | ||
| - Guard the existing standard block with `if not IsHandled then`, so a subscriber that sets | ||
| `IsHandled := true` fully replaces the standard decision. | ||
|
|
||
| Illustrative shape (final event name and signature must follow BC event conventions): | ||
|
|
||
| ```al | ||
| IsHandled := false; | ||
| // new integration event raised here, passing Service Header, IsHandled, and the posted document numbers | ||
| if not IsHandled then | ||
| if Invoice then | ||
| if ServiceHeader."Document Type" in [ServiceHeader."Document Type"::Order, ServiceHeader."Document Type"::Invoice] then begin | ||
| ServInvoiceNo := ServDocumentsMgt.PrepareInvoiceHeader(Window); | ||
| // ... | ||
| end else begin | ||
| ServCrMemoNo := ServDocumentsMgt.PrepareCrMemoHeader(Window); | ||
| // ... | ||
| end; | ||
| ``` | ||
|
|
||
| ## Scope | ||
|
|
||
| - This codeunit exists in the W1 base layer **and** in the IT and NA layers, which keep their own | ||
| copies of the same posting logic. The change must be applied consistently to every layer that has a | ||
| copy: | ||
| - `App/Layers/W1/BaseApp/Service/Posting/ServicePost.Codeunit.al` | ||
| - `App/Layers/IT/BaseApp/Service/Posting/ServicePost.Codeunit.al` | ||
| - `App/Layers/NA/BaseApp/Service/Posting/ServicePost.Codeunit.al` | ||
| - No other layers keep a copy of this codeunit, so only these three files change. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # Extensibility request: extension point in report 10135 "Item Sales Statistics" before the "print only if sales" skip | ||
|
|
||
| ## Why this change is needed | ||
|
|
||
| Report 10135 "Item Sales Statistics" (an NA local report) computes an item's `Sales (Qty.)` | ||
| FlowField in the `Item` data item's `OnAfterGetRecord` trigger, and, in the same statement block, | ||
| immediately decides whether to skip the record based on the report's `PrintOnlyIfSales` option: | ||
|
|
||
| ```al | ||
| CalcFields("Sales (Qty.)", "Sales (LCY)", "COGS (LCY)"); | ||
| if ("Sales (Qty.)" = 0) and PrintOnlyIfSales then | ||
| CurrReport.Skip(); | ||
| ``` | ||
|
|
||
| `PrintOnlyIfSales` is a private global with no accessible getter, and the skip decision is made | ||
| before any extension trigger runs. An extension that computes its own sales figure (for example an | ||
| alternate unit-of-measure total from its own ledger sums) therefore cannot influence whether a record | ||
| is printed: an item the extension considers to have real sales can be silently skipped, and an item | ||
| the extension considers zero-sales can still print. | ||
|
|
||
| We need an extension point that exposes the already-computed skip decision so a subscriber can | ||
| override it in either direction. | ||
|
|
||
| ## Requested change | ||
|
|
||
| Add a new integration event to report 10135 "Item Sales Statistics", raised in the `Item` data | ||
| item's `OnAfterGetRecord` trigger **immediately after** the existing `CalcFields` call and | ||
| **before** the `PrintOnlyIfSales` skip check. | ||
|
|
||
| - Introduce a local `SkipRecord: Boolean` variable in the trigger. | ||
| - Assign it the existing skip condition (`("Sales (Qty.)" = 0) and PrintOnlyIfSales`). | ||
| - Raise the new event passing the `Item` record and `PrintOnlyIfSales`, and the `SkipRecord` flag by | ||
| reference so a subscriber can override it. | ||
| - Replace the original condition with `if SkipRecord then CurrReport.Skip();`. | ||
|
|
||
| Illustrative shape (final event name and signature must follow BC event conventions): | ||
|
|
||
| ```al | ||
| SetRange("Variant Filter"); | ||
| CalcFields("Sales (Qty.)", "Sales (LCY)", "COGS (LCY)"); | ||
| SkipRecord := ("Sales (Qty.)" = 0) and PrintOnlyIfSales; | ||
| // new integration event raised here, passing Item, PrintOnlyIfSales, and SkipRecord (var) | ||
| if SkipRecord then | ||
| CurrReport.Skip(); | ||
| ``` | ||
|
|
||
| ## Scope | ||
|
|
||
| - File: `App/Layers/NA/BaseApp/Local/Inventory/Reports/ItemSalesStatistics.Report.al` | ||
| - This is an NA local report that exists only in the NA layer, so the change lives in the NA layer | ||
| alone (no other country/region layer counterparts to propagate to). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,7 +74,7 @@ def run_copilot_agent( | |
| "--allow-all-tools", # required for non-interactive mode | ||
| "--disable-builtin-mcps", | ||
| f"--model={model}", | ||
| "--log-level=debug", | ||
| "--log-level=all", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think log level
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — reverted back to --log-level=debug\ (the original) in bffe360. |
||
| f"--log-dir={output_dir.resolve()}", | ||
| f"--prompt={prompt.replace('\r', '').replace('\n', ' ')}", | ||
| ] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.