Skip to content

Extract AU Withholding Tax code#9258

Open
Alexander-Ya wants to merge 13 commits into
mainfrom
bugs/Extract-AU-Withholding-Tax-code
Open

Extract AU Withholding Tax code#9258
Alexander-Ya wants to merge 13 commits into
mainfrom
bugs/Extract-AU-Withholding-Tax-code

Conversation

@Alexander-Ya

@Alexander-Ya Alexander-Ya commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What & why

Linked work

Fixes AB#636621

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome (required — be specific: scenarios, commands, screenshots for UI changes)

Risk & compatibility

@Alexander-Ya Alexander-Ya requested a review from a team July 8, 2026 15:22
@github-actions

This comment was marked as resolved.

@github-actions github-actions Bot added the needs-approval Workflow runs require maintainer approval to start label Jul 8, 2026
@github-actions

This comment was marked as outdated.

@github-actions github-actions Bot removed the needs-approval Workflow runs require maintainer approval to start label Jul 8, 2026
Comment thread src/Layers/APAC/BaseApp/Finance/GeneralLedger/Journal/GenJournalLine.Table.al Outdated
Comment thread src/Layers/APAC/BaseApp/Purchases/Document/PurchaseLine.Table.al
@github-actions

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@github-actions github-actions Bot added this to the Version 29.0 milestone Jul 10, 2026
Comment thread src/Layers/W1/BaseApp/Sales/Posting/SalesPost.Codeunit.al
@github-actions

This comment was marked as outdated.

Comment thread src/Layers/W1/BaseApp/Sales/Posting/SalesPost.Codeunit.al
Caption = 'Purch. - Tax Invoice';
Image = "Report";
RunObject = Report "Purch. - Tax Invoice";
ToolTip = 'Create a new purchase tax credit invoice.';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility} \quad \color{gray}{\texttt{\small Iteration\ 9}}$

The tooltip on 'Purch.

  • Tax Invoice' contradicts the action caption and RunObject by saying it creates a "purchase tax credit invoice." This mislabels the action in the Role Center. Update the tooltip to describe a purchase tax invoice instead.
Suggested change
ToolTip = 'Create a new purchase tax credit invoice.';
ToolTip = 'Create a new purchase tax invoice.';

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

TotalWHTAmountToBeDeductedLCY: Decimal;
TotalWHTAmtTobeDeducted: Decimal;
TotalWHTAmountToBeDeductedACY: Decimal;
begin

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 9}}$

The OnPrepareLineOnBeforeSetInvoiceDiscountPosting subscriber queries Purch.

Inv. Header and then WHT Entry before checking PurchLine."Prepayment Line". This subscriber runs while preparing purchase lines, so non-prepayment lines still pay those database round-trips. Add the cheap Prepayment Line guard first and only run the queries when it passes.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    begin
        if not PurchLine."Prepayment Line" then
            exit;

        PrepmtPurchInvHeader.Reset();
        PrepmtPurchInvHeader.SetRange("Prepayment Order No.", PurchLine."Document No.");
        PrepmtPurchInvHeader.SetRange("Prepayment Invoice", true);
        if PrepmtPurchInvHeader.FindSet() then
            repeat
                PrepmtWHTEntry.SetRange("Document Type", PrepmtWHTEntry."Document Type"::Invoice);
                PrepmtWHTEntry.SetRange("Document No.", PrepmtPurchInvHeader."No.");
                PrepmtWHTEntry.SetRange("Gen. Bus. Posting Group", GenPostingSetup."Gen. Bus. Posting Group");
                PrepmtWHTEntry.SetRange("Gen. Prod. Posting Group", GenPostingSetup."Gen. Prod. Posting Group");
                if PrepmtWHTEntry.FindSet() then
                    repeat
                        TotalWHTAmountToBeDeductedLCY := TotalWHTAmountToBeDeductedLCY + PrepmtWHTEntry."Unrealized Amount (LCY)";
                        TotalWHTAmtTobeDeducted := TotalWHTAmtTobeDeducted + PrepmtWHTEntry."Unrealized Amount";
                    until PrepmtWHTEntry.Next() = 0;
            until PrepmtPurchInvHeader.Next() = 0;

        GLSetup.Get();
        if GLSetup."Additional Reporting Currency" <> '' then
            TotalWHTAmountToBeDeductedACY :=
              CurrExchRate.ExchangeAmtLCYToFCY(
                PurchHeader."Posting Date", GLSetup."Additional Reporting Currency",
                TotalWHTAmtTobeDeducted, 0);

        TotalAmount := TotalAmount + TotalWHTAmountToBeDeductedLCY;
        TotalAmountACY := TotalAmountACY + TotalWHTAmountToBeDeductedACY;
    end;

Knowledge:

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

action("Posted Purch. Tax Credit Memo")
{
ApplicationArea = Basic, Suite;
Caption = 'Posted Purch. Tax Credit Memo';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility} \quad \color{gray}{\texttt{\small Iteration\ 10}}$

The action caption `Posted Purch.

Tax Credit Memocontains a double space, which shows up as a visible typo in the Role Center. Change the caption text toPosted Purch. Tax Credit Memo`.

Suggested change
Caption = 'Posted Purch. Tax Credit Memo';
Caption = 'Posted Purch. Tax Credit Memo';

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

Comment on lines +1379 to +1384
action("AU/NZ Statement")
{
ApplicationArea = Basic, Suite;
Caption = 'AU/NZ Statement';
RunObject = Report "AU/NZ Statement";
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility} \quad \color{gray}{\texttt{\small Iteration\ 10}}$

The newly added AU/NZ Statement action has no ToolTip, so this Role Center entry loses the hover/help text that the same report action already provides elsewhere.

Add the standard report tooltip here as well.

Suggested change
action("AU/NZ Statement")
{
ApplicationArea = Basic, Suite;
Caption = 'AU/NZ Statement';
RunObject = Report "AU/NZ Statement";
}
action("AU/NZ Statement")
{
ApplicationArea = Basic, Suite;
Caption = 'AU/NZ Statement';
RunObject = Report "AU/NZ Statement";
ToolTip = 'Print the report of accounts receivable and accounts payable based on the selections you make in the report request window.';
}

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

Comment on lines +29 to +38
PrepmtPurchInvHeader.Reset();
PrepmtPurchInvHeader.SetRange("Prepayment Order No.", PurchLine."Document No.");
PrepmtPurchInvHeader.SetRange("Prepayment Invoice", true);
if PrepmtPurchInvHeader.FindSet() then
repeat
PrepmtWHTEntry.SetRange("Document Type", PrepmtWHTEntry."Document Type"::Invoice);
PrepmtWHTEntry.SetRange("Document No.", PrepmtPurchInvHeader."No.");
PrepmtWHTEntry.SetRange("Gen. Bus. Posting Group", GenPostingSetup."Gen. Bus. Posting Group");
PrepmtWHTEntry.SetRange("Gen. Prod. Posting Group", GenPostingSetup."Gen. Prod. Posting Group");
if PrepmtWHTEntry.FindSet() then

Copy link
Copy Markdown
Contributor

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\ 10}}$

Both added FindSet() loops read only a small subset of fields from wide records (`Purch.

Inv. HeaderandWHT Entry), but no SetLoadFields(...)` is declared before the reads. That causes full-row materialization on every iteration instead of loading just the fields used by the accumulation logic.

Suggested change
PrepmtPurchInvHeader.Reset();
PrepmtPurchInvHeader.SetRange("Prepayment Order No.", PurchLine."Document No.");
PrepmtPurchInvHeader.SetRange("Prepayment Invoice", true);
if PrepmtPurchInvHeader.FindSet() then
repeat
PrepmtWHTEntry.SetRange("Document Type", PrepmtWHTEntry."Document Type"::Invoice);
PrepmtWHTEntry.SetRange("Document No.", PrepmtPurchInvHeader."No.");
PrepmtWHTEntry.SetRange("Gen. Bus. Posting Group", GenPostingSetup."Gen. Bus. Posting Group");
PrepmtWHTEntry.SetRange("Gen. Prod. Posting Group", GenPostingSetup."Gen. Prod. Posting Group");
if PrepmtWHTEntry.FindSet() then
PrepmtPurchInvHeader.Reset();
PrepmtPurchInvHeader.SetLoadFields("No.");
PrepmtPurchInvHeader.SetRange("Prepayment Order No.", PurchLine."Document No.");
PrepmtPurchInvHeader.SetRange("Prepayment Invoice", true);
if PrepmtPurchInvHeader.FindSet() then
repeat
PrepmtWHTEntry.SetLoadFields("Unrealized Amount (LCY)", "Unrealized Amount");
PrepmtWHTEntry.SetRange("Document Type", PrepmtWHTEntry."Document Type"::Invoice);
PrepmtWHTEntry.SetRange("Document No.", PrepmtPurchInvHeader."No.");
PrepmtWHTEntry.SetRange("Gen. Bus. Posting Group", GenPostingSetup."Gen. Bus. Posting Group");
PrepmtWHTEntry.SetRange("Gen. Prod. Posting Group", GenPostingSetup."Gen. Prod. Posting Group");
if PrepmtWHTEntry.FindSet() then

Knowledge:

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

OnAfterInitGenJnlLineAmountFieldsFromTotalLines(GenJnlLine, SalesHeader, TotalSalesLine, TotalSalesLineLCY);
end;

[IntegrationEvent(false, false)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 10}}$

The new OnAfterInitGenJnlLineAmountFieldsFromTotalLines publisher is raised inside sales invoice posting without [CommitBehavior(CommitBehavior::Ignore)].

A subscriber can call Commit() during posting and persist partial state before later posting code fails; add CommitBehavior::Ignore on the publisher to preserve rollback semantics.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

[CommitBehavior(CommitBehavior::Ignore)]
[IntegrationEvent(false, false)]
local procedure OnAfterInitGenJnlLineAmountFieldsFromTotalLines(var GenJnlLine: Record "Gen. Journal Line"; var SalesHeader: Record "Sales Header"; var TotalSalesLine: Record "Sales Line"; var TotalSalesLineLCY: Record "Sales Line")

Knowledge:

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

WHTManagement: Codeunit WHTManagement;

[EventSubscriber(ObjectType::Codeunit, Codeunit::"Sales-Post", 'OnPostInvoiceOnAfterPostLines', '', true, false)]
local procedure OnPostInvoiceOnAfterPostLines(var SalesHeader: Record "Sales Header"; SrcCode: Code[10]; GenJnlLineDocType: Enum "Gen. Journal Document Type"; GenJnlLineDocNo: Code[20]; GenJnlLineExtDocNo: Code[35]; var GenJnlPostLine: Codeunit "Gen. Jnl.-Post Line"; TotalAmount: Decimal; var TempSalesLineGlobal: Record "Sales Line" temporary; var SalesInvoiceHeader: Record "Sales Invoice Header"; var SalesCrMemoHeader: Record "Sales Cr.Memo Header"; var TotalSalesLineLCY: Record "Sales Line" temporary)

Copy link
Copy Markdown
Contributor

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\ 10}}$

TotalSalesLineLCY is declared as Record "Sales Line" temporary in this new codeunit, but the parameter name does not start with Temp.

The same non-conforming temporary-record name is repeated in the subscriber signature, PostWHT, and InsertGenJournalWHT, which hides the fact that the record is in-memory only.

Knowledge:

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

begin
GLSetup.Get();
if GLSetup."Enable WHT" and (not GLSetup."Enable GST (Australia)") then
PostWHT(

Copy link
Copy Markdown
Contributor

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\ 10}}$

This new codeunit calls its own procedures as bare identifiers (PostWHT, InsertGenJournalWHT, CheckWHTApplication, UpdateTaxForPostedDoc) instead of qualifying them with this..

In a codeunit, this. is the required self-reference convention and makes the local call target unambiguous.

Knowledge:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants