Deliverable 629871: [master] [UK] Payment Practices updates

[AleksanderGladkov]

What was done

• Added a new Reporting Scheme dimension (Standard / Dispute & Retention / Small Business) so the app can support country-specific payment practice rules without affecting existing W1/FR users.
• Implemented the UK (Dispute & Retention) scheme: dispute tracking, supply chain finance payment dates, and "% overdue due to dispute" reporting.
• Implemented the AU/NZ (Small Business) scheme: filters reporting to small-business suppliers and adds invoice count / value per period.
• Auto-detects the right scheme from the environment country on new reports, with an event for partners to override.
• Upgrade logic backfills the scheme on existing headers, so nothing breaks for current users.
• Surfaced the new fields and scheme selector on the relevant Payment Practice pages.
• Extracted shared test helpers into a new Test Library app and expanded unit tests across all three schemes.

Fixes AB#629871

Fixes AB#626294

[github-actions]

AL Documentation Audit

Documentation gaps were detected in the following apps:

• Payment-Practices-Test-Library: 0% documentation coverage
• Payment-Practices-Tests: 0% documentation coverage
• Payment-Practices: 10% documentation coverage

To generate documentation, run /al-docs init or /al-docs update using GitHub Copilot CLI or Claude Code.
This review is for awareness to help keep documentation in sync with code changes. It is okay to dismiss this request.

[github-actions]

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.

---

🔄 How to trigger a new status check:

1. 📤 Push a new commit to the PR branch, or
2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

[ventselartur]

Please, review my comments

[github-actions]

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

Dispute & Retention page fields missing ToolTip

The four new fields inside the "Dispute and Retention" group ("Total Number of Payments", "Total Amount of Payments", "Total Amt. of Overdue Payments", "Pct Overdue Due to Dispute") are added to the page without explicit ToolTip properties. AL code-quality rules require every visible page field to declare a ToolTip; absence causes build warnings and degrades discoverability in the UI.

Recommendation:

• Add a ToolTip property to each of the four new page fields, either matching the table-field tooltip or providing a page-context-specific description.

Suggested change

	group("Dispute and Retention")
	field("Total Number of Payments"; Rec."Total Number of Payments")
	{
	ToolTip = 'Specifies the total number of payments made during the reporting period.';
	}
	field("Total Amount of Payments"; Rec."Total Amount of Payments")
	{
	ToolTip = 'Specifies the total value of payments made during the reporting period.';
	}
	field("Total Amt. of Overdue Payments"; Rec."Total Amt. of Overdue Payments")
	{
	ToolTip = 'Specifies the total value of payments not made within the agreed payment terms.';
	}
	field("Pct Overdue Due to Dispute"; Rec."Pct Overdue Due to Dispute")
	{
	ToolTip = 'Specifies the percentage of late payments attributable to disputes.';
	}

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

[github-actions]

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

Drill-down shows all invoices, not small business ones

ShowSmallBusinessVendorLedgerEntries filters Vendor Ledger Entries only by document type and posting date, showing all vendor invoices in the period. A user drilling down from the "Pct Small Business Payments" field reasonably expects to see only small-business vendor entries, but will instead see every invoice in the period.

Recommendation:

• Apply an additional filter to restrict the page to vendors whose associated Company Size has "Small Business" = true, or open the Payment Practice Data List page pre-filtered on the small-business data rows (as the other drill-downs do).

Suggested change

	VendorLedgerEntry: Record "Vendor Ledger Entry";
	local procedure ShowSmallBusinessVendorLedgerEntries()
	var
	PaymentPracticeData: Record "Payment Practice Data";
	begin
	PaymentPracticeData.SetRange("Header No.", Rec."No.");
	PaymentPracticeData.SetRange("Source Type", PaymentPracticeData."Source Type"::Vendor);
	// Show data already filtered to small-business vendors (see SmallBusHandler.UpdatePaymentPracData)
	Page.RunModal(Page::"Payment Practice Data List", PaymentPracticeData);
	end;

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

[github-actions]

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

Invoice Count and Value page fields missing ToolTip

The two new fields "Invoice Count" and "Invoice Value" added to the Payment Practice Lines page lack ToolTip properties, violating AL page-field requirements and resulting in build warnings.

Recommendation:

• Add ToolTip to both new page fields.

field("Invoice Count"; Rec."Invoice Count")
{
    Editable = false;
    ToolTip = 'Specifies the number of invoices in this period.';
}
field("Invoice Value"; Rec."Invoice Value")
{
    Editable = false;
    ToolTip = 'Specifies the total value of invoices in this period.';
}

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

Backfill overwrites intentionally-Standard scheme headers

BackfillReportingScheme filters on "Reporting Scheme" = 0 (Standard) and overwrites them all with the environment-detected scheme. On a non-GB tenant, DetectReportingScheme() returns Standard (0), so ModifyAll is a no-op. On a GB tenant it returns Dispute & Retention, which incorrectly overwrites any header that was explicitly set to Standard — but since the field is Editable = false, all pre-upgrade records have value 0 by default, so the intent seems correct. However, the filter comment and intent is misleading and fragile for future enum additions where 0 might be legitimately used.

Recommendation:

• Add a comment explaining the intent, or use a dedicated sentinel value / a Boolean flag "Reporting Scheme Initialised" to distinguish unset from explicitly-set-to-Standard.

Suggested change

	// Only backfill headers that were inserted before the Reporting Scheme field existed
	// (all pre-upgrade headers have value 0 = Standard because the field did not exist).
	PaymentPracticeHeader.SetRange("Reporting Scheme", PaymentPracticeHeader."Reporting Scheme"::Standard);
	PaymentPracticeHeader.ModifyAll("Reporting Scheme", ReportingScheme);

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

[github-actions]

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

AUNZ default periods changed without data migration

The AUNZ default payment periods were consolidated from five bands (0–21, 22–30, 31–60, 61–120, 121+) to three (0–30, 31–60, 61+). Existing AUNZ tenants who have already generated Payment Practice Line rows referencing the old period codes (P0_21, P22_30, P61_90, P121+) will have orphaned lines because the old Payment Period records are not migrated or deleted and no upgrade codeunit handles this change.

Recommendation:

• Add a data upgrade step in UpgradePaymentPractices (or a separate upgrade codeunit) that deletes or remaps the old AUNZ period records and regenerates any affected Payment Practice Line rows, or at minimum document that existing AUNZ data must be re-generated.

// In UpgradePaymentPractices.BackfillReportingScheme or a new procedure:
local procedure UpgradeAUNZPeriods()
var
    PaymentPeriod: Record "Payment Period";
begin
    if UpgradeTag.HasUpgradeTag(GetAUNZPeriodsUpgradeTag()) then
        exit;
    // Remove obsolete AUNZ periods so InsertDefaultPeriods_AUNZ re-creates the new ones
    for each obsoleteCode in ['P0_21', 'P22_30', 'P61_90', 'P121+'] do
        if PaymentPeriod.Get(obsoleteCode) then
            PaymentPeriod.Delete();
    UpgradeTag.SetUpgradeTag(GetAUNZPeriodsUpgradeTag());
end;

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

Five+ sequential full-table scans per report run

CalculateHeaderTotals makes at least five independent calls that each iterate PaymentPracticeData in full: GetModePaymentTime, GetModePaymentTimeMin, GetModePaymentTimeMax, GetPaymentTimeStatistics, and GetPctPeppolEnabled. For large reporting periods this multiplies the DB I/O by 5×.

Recommendation:

• Consolidate into a single pass that accumulates counts, sums, the sorted list for statistics, per-vendor mode values, and the PEPPOL GLN dictionary simultaneously. The GetPctSmallBusinessPayments call to raw VLE should be collapsed into the same pass using PaymentPracticeData.

Suggested change

	PaymentPracticeHeader."Mode Payment Time" := PaymentPracticeMath.GetModePaymentTime(PaymentPracticeData);
	// Combine all metrics into a single FindSet loop or split into at most two passes
	// (one for mode/statistics over PaymentPracticeData, one for PEPPOL GLN lookups).

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

[github-actions]

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

AU/NZ period codes changed, existing lines orphaned

InsertDefaultPeriods_AUNZ is reduced from 5 buckets (P0_21, P22_30, P31_60, P61_90, P121+) to 3 (P0_30, P31_60, P61+). The new "Restore Defaults" action deletes all periods and recreates them. Any PaymentPracticeLine records previously generated for AU/NZ companies that reference the removed period codes (P0_21, P22_30, P61_90, P121+) will have dangling foreign-key references to non-existent PaymentPeriod records.

Recommendation:

• Add an upgrade step in UpgradePaymentPractices that deletes PaymentPracticeHeader (and cascades to PaymentPracticeLines) for AU/NZ companies, or at minimum deletes all PaymentPracticeLine records referencing the removed period codes. Alternatively, keep the old period codes as deprecated records so existing lines remain valid.

// In UpgradePaymentPractices.OnUpgradePerCompany(), add:
local procedure CleanupObsoleteAUNZPeriodLines()
var
    PaymentPracticeLine: Record "Payment Practice Line";
begin
    if UpgradeTag.HasUpgradeTag(GetAUNZPeriodsCleanupTag()) then exit;
    PaymentPracticeLine.SetFilter("Payment Period Code", 'P0_21|P22_30|P61_90|P121+');
    PaymentPracticeLine.DeleteAll();
    UpgradeTag.SetUpgradeTag(GetAUNZPeriodsCleanupTag());
end;

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

Reporting Scheme field visible=false but no ToolTip

The new 'Reporting Scheme' field on line 28 has no ToolTip property. Even fields with Visible = false should carry a ToolTip so that personalisation and page inspection tools display meaningful text.

Recommendation:

• Add a ToolTip matching the table field definition.

field("Reporting Scheme"; Rec."Reporting Scheme")
{
    Visible = false;
    ToolTip = 'Specifies which reporting scheme is used, such as Standard, Dispute & Retention, or Small Business.';
}

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

ShowVendorInvoicesInReportPeriod exposes unfiltered entries

ShowVendorInvoicesInReportPeriod opens Vendor Ledger Entries filtered only by document type and posting date, ignoring Exclude from Pmt. Practices and vendor-level filters applied during data generation. Users may see invoices that were intentionally excluded from the report.

Recommendation:

• Add the same exclusion filters used during data building (e.g., Vendor."Exclude from Pmt. Practices" = false) or link the drilldown to the Payment Practice Data list filtered by the header, which already represents the clean dataset.

local procedure ShowVendorInvoicesInReportPeriod()
var
    PaymentPracticeData: Record "Payment Practice Data";
begin
    PaymentPracticeData.SetRange("Header No.", Rec."No.");
    PaymentPracticeData.SetRange("Source Type", PaymentPracticeData."Source Type"::Vendor);
    Page.RunModal(Page::"Payment Practice Data List", PaymentPracticeData);
end;

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

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

Statistical methods load all rows into memory

GetModePaymentTime, GetClosedInvoicePaymentTimes, and GetModesPerVendor iterate every closed invoice and accumulate values into AL List of [Integer] in memory. For high-volume companies this may exhaust available memory or exceed runtime limits.

Recommendation:

• Consider computing mode/median/percentiles incrementally or via sorted key traversal rather than loading the entire dataset into an in-memory list. At minimum, document the expected scale limitations.

Suggested change

	if PaymentPracticeData.FindSet() then
	// For large datasets, consider computing these statistics with a sorted key scan
	// instead of loading all records into a List of [Integer].

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

[github-actions]

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

Vendor.Get loads all fields unnecessarily

Vendor.Get() inside the GetPctSmallBusinessPayments loop reads all vendor fields from the database. Only Company Size Code is consumed.

Recommendation:

• Call Vendor.SetLoadFields('Company Size Code') before the loop so only the required field is fetched.

Suggested change

	if Vendor.Get(VendorLedgerEntry."Vendor No.") then
	Vendor.SetLoadFields("Company Size Code");
	if VendorLedgerEntry.FindSet() then
	repeat
	if Vendor.Get(VendorLedgerEntry."Vendor No.") then
	...

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

[github-actions]

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

ShowVendorInvoicesInReportPeriod not scoped to header vendors

ShowVendorInvoicesInReportPeriod opens the Vendor Ledger Entries page filtered only by Document Type and Posting Date range. It shows invoices from ALL vendors in the period, not just the vendors included in the current Payment Practice Header. A user drilling down from a Payment Practice scoped to specific vendors may inadvertently see invoices from unrelated vendors.

Recommendation:

• Filter the Vendor Ledger Entry list to the vendors actually included in the Payment Practice Header's generated data by joining through PaymentPracticeData.

Suggested change

	VendorLedgerEntry.SetRange("Document Type", VendorLedgerEntry."Document Type"::Invoice);
	local procedure ShowVendorInvoicesInReportPeriod()
	var
	VendorLedgerEntry: Record "Vendor Ledger Entry";
	PaymentPracticeData: Record "Payment Practice Data";
	begin
	PaymentPracticeData.SetRange("Header No.", Rec."No.");
	PaymentPracticeData.SetRange("Source Type", PaymentPracticeData."Source Type"::Vendor);
	if PaymentPracticeData.FindSet() then
	repeat
	VendorLedgerEntry.SetRange("Entry No.", PaymentPracticeData."Invoice Entry No.");
	until PaymentPracticeData.Next() = 0;
	VendorLedgerEntry.SetRange("Document Type", VendorLedgerEntry."Document Type"::Invoice);
	VendorLedgerEntry.SetRange("Posting Date", Rec."Starting Date", Rec."Ending Date");
	Page.RunModal(Page::"Vendor Ledger Entries", VendorLedgerEntry);
	end;

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

[github-actions]

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

PrepareLayout never resets to default report layout

PrepareLayout only sets the layout when the scheme is Small Business. If a user previously ran a Small Business report, DesignTimeReportSelection has set the layout to PaymentPractice_SmallBusinessLayout. On the next run with a Standard or Dispute & Retention scheme, the layout is not reset, so the wrong template may be applied.

Recommendation:

• Use an else branch (or a case statement) to explicitly set the default layout for Standard and Dispute & Retention schemes.

Suggested change

	local procedure PrepareLayout(PaymentPracticeLinesAggregator: Interface PaymentPracticeLinesAggregator; ReportingScheme: Enum "Paym. Prac. Reporting Scheme")
	local procedure PrepareLayout(PaymentPracticeLinesAggregator: Interface PaymentPracticeLinesAggregator; ReportingScheme: Enum "Paym. Prac. Reporting Scheme")
	var
	DesignTimeReportSelection: Codeunit "Design-time Report Selection";
	begin
	PaymentPracticeLinesAggregator.PrepareLayout();
	case ReportingScheme of
	ReportingScheme::"Small Business":
	begin
	DesignTimeReportSelection.SetSelectedLayout('PaymentPractice_SmallBusinessLayout');
	FeatureTelemetry.LogUsage('0000KSU', 'Payment Practices', 'Small Business layout used.');
	end;
	else
	DesignTimeReportSelection.SetSelectedLayout('PaymentPractice_StandardLayout');
	end;
	end;

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Upgrade} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

AU/NZ period consolidation has no data upgrade

InsertDefaultPeriods_AUNZ() now creates 3 periods with new codes (P0_30, P31_60, P61+) replacing the 5 old ones (P0_21, P22_30, P31_60, P61_90, P121+). Existing Payment Practice Line records referencing the old period codes will have dangling foreign keys after an upgrade, and historical reports will be broken. There is no upgrade code to migrate or delete the obsolete lines.

Recommendation:

• Add upgrade logic that deletes or migrates existing AU/NZ Payment Practice Line records that reference the removed period codes, and updates or re-generates affected Payment Practice Header records.

// In UpgradePaymentPractices.Codeunit.al, add:
local procedure UpgradeAUNZPeriodCodes()
var
    PaymentPracticeLine: Record "Payment Practice Line";
begin
    if UpgradeTag.HasUpgradeTag(GetAUNZPeriodsUpgradeTag()) then
        exit;
    PaymentPracticeLine.SetFilter("Payment Period Code", 'P0_21|P22_30|P61_90|P121+');
    PaymentPracticeLine.DeleteAll();
    UpgradeTag.SetUpgradeTag(GetAUNZPeriodsUpgradeTag());
end;

Line mapping was unavailable, so this was posted as an issue comment.

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Upgrade} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

OnInsert unconditionally overwrites Reporting Scheme

DetectReportingScheme() is called in OnInsert and always sets "Reporting Scheme" from the environment. Any code that assigns the scheme before calling Insert() — including the new test library's CreatePaymentPracticeHeader and CreatePaymentPracticeHeaderWithScheme overloads — will silently have its value overwritten, making explicit scheme assignments impossible.

Recommendation:

• Only auto-detect when the field has not been set explicitly, i.e. guard with if "Reporting Scheme" = "Reporting Scheme"::Standard then.

Suggested change

	"Reporting Scheme" := PaymentPractices.DetectReportingScheme();
	local procedure DetectReportingScheme()
	var
	PaymentPractices: Codeunit "Payment Practices";
	begin
	if "Reporting Scheme" <> "Reporting Scheme"::Standard then
	exit; // Already set explicitly by the caller
	"Reporting Scheme" := PaymentPractices.DetectReportingScheme();
	end;

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