Skip to content

Bug 634267: [Subcontracting] Transfer Order Reopen not persisting from factbox drill-down (Bug 634267)#8170

Open
ChethanT wants to merge 1 commit into
mainfrom
bugs/Subcontracting/634267-TOOpenNotPersisted
Open

Bug 634267: [Subcontracting] Transfer Order Reopen not persisting from factbox drill-down (Bug 634267)#8170
ChethanT wants to merge 1 commit into
mainfrom
bugs/Subcontracting/634267-TOOpenNotPersisted

Conversation

@ChethanT
Copy link
Copy Markdown
Contributor

@ChethanT ChethanT commented May 15, 2026

Problem

When a Transfer Order is opened from the Subcontracting Details Factbox (e.g. clicking No. of Transfer Orders on a Purchase Order), actions like Reopen that modify Rec directly appear to succeed on screen but the change is silently lost when the page closes.

Root Cause

ShowTransferOrdersAndReturnOrder in SubcPurchFactboxMgmt collected Transfer Headers using Mark/MarkedOnly and passed that marked record directly to PageManagement.PageRun/PageRunList. When the Transfer Order page opened bound to a marked record, operations that modify Rec (e.g. Reopen → Rec.Validate(Status, Open); Rec.Modify()) wrote to the marked record set in memory rather than the real database table. The change was discarded when the page closed.

Actions like Post were unaffected because they call TransferHeader.Get() internally on a separate variable, bypassing Rec.

Fix

Introduced a separate TransferHeaderToOpen record variable that is populated from the real database:

  • Single record: TransferHeaderToOpen.Get(TransferHeader.\"No.\") fetches the real DB record, then PageManagement.PageRun(TransferHeaderToOpen).
  • Multiple records: Uses SelectionFilterManagement.GetSelectionFilterForTransferHeader to build a "No." filter from the marked set, applies it to TransferHeaderToOpen, then PageManagement.PageRunList(TransferHeaderToOpen).

Test

Added FactboxDrilldownTransferOrderReopenPersists:

  1. Sets up subcontracting with transfer components
  2. Creates and releases a Transfer Order
  3. Opens it via ShowTransferOrdersAndReturnOrder (the factbox drill-down path)
  4. Page handler invokes Reopen
  5. Asserts the Transfer Order status persists as Open after the page closes

AB#634267

…g 634267)

ShowTransferOrdersAndReturnOrder passed a MarkedOnly-filtered record to
PageManagement.PageRun/PageRunList. When the Transfer Order page opened
bound to this marked record, actions like Reopen that modify Rec directly
wrote to the marked record set rather than the real database, so changes
were silently lost when the page closed.

The fix uses a separate TransferHeaderToOpen record variable:
- Single record: Get() fetches the real DB record before PageRun.
- Multiple records: SelectionFilterManagement builds a No. filter
  applied to a clean record before PageRunList.

Added test FactboxDrilldownTransferOrderReopenPersists that releases a
transfer order, opens it via ShowTransferOrdersAndReturnOrder, performs
Reopen in the page handler, and asserts the status persists as Open.

AB#634267

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ChethanT ChethanT requested a review from a team as a code owner May 15, 2026 14:58
@ChethanT ChethanT requested a review from kmhansen May 15, 2026 14:58
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label May 15, 2026
@github-actions github-actions Bot modified the milestone: Version 29.0 May 15, 2026
@ChethanT ChethanT added the Subcontracting Subcontracting related activities label May 15, 2026
NoOfTransferHeaders > 1:
PageManagement.PageRunList(TransferHeader);
begin
TransferHeaderToOpen.SetFilter("No.", SelectionFilterMgt.GetSelectionFilterForTransferHeader(TransferHeader));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Filter string may grow unbounded for large sets

GetSelectionFilterForTransferHeader iterates all marked TransferHeader records and encodes their "No." values into a pipe-separated filter string. If many transfer orders are marked (e.g., in a high-volume manufacturing environment), the resulting filter string could become very long, risk hitting AL text-length limits, and generate a complex SQL IN-style predicate that performs worse than the previous marked-record approach.

Recommendation:

  • Consider guarding with a reasonable upper bound check before calling GetSelectionFilterForTransferHeader, or fall back to the marked-record approach when NoOfTransferHeaders exceeds a configurable threshold. Alternatively, document the expected max cardinality in a comment to make the assumption explicit.
Suggested change
TransferHeaderToOpen.SetFilter("No.", SelectionFilterMgt.GetSelectionFilterForTransferHeader(TransferHeader));
NoOfTransferHeaders > 1:
begin
// Note: NoOfTransferHeaders is bounded by subcontracting purchase order scope;
// a single PO is unlikely to have more than a few dozen linked transfer orders.
TransferHeaderToOpen.SetFilter("No.", SelectionFilterMgt.GetSelectionFilterForTransferHeader(TransferHeader));
PageManagement.PageRunList(TransferHeaderToOpen);
end;

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

using Microsoft.Purchases.History;
using Microsoft.Utilities;
using System.Reflection;
using System.Text;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Unused namespace import added

The using System.Text; statement was added in this PR but no type from the System.Text namespace (such as TextBuilder) is referenced anywhere in the file. The new SelectionFilterManagement codeunit does not belong to System.Text, so this import is dead code that can trigger AL compiler warnings about unused using directives.

Recommendation:

  • Remove the using System.Text; line. If SelectionFilterManagement requires an explicit namespace import to resolve without ambiguity, add its correct namespace instead (e.g., using Microsoft.Foundation.Navigate; if needed).
Suggested change
using System.Text;
using System.Reflection;
// remove: using System.Text;

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

@ChethanT ChethanT changed the title Fix: Transfer Order Reopen not persisting from factbox drill-down (Bug 634267) Bug 634267: [Subcontracting] Transfer Order Reopen not persisting from factbox drill-down (Bug 634267) May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Subcontracting Subcontracting related activities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant