Skip to content

Extract AU Withholding Tax code#9258

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

Extract AU Withholding Tax code#9258
Alexander-Ya wants to merge 5 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 #

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

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234'

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 4 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain Findings Knowledge-backed Agent Inline Fallback
Agent 1 0 1 0 0

Totals: 0 knowledge-backed · 1 agent findings.

Orchestrator pre-filter (13 file(s) excluded)

  • layer-disabled (knowledge) : 13 file(s)

Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.

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

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🔴\ Critical\ Severity\ —\ Upgrade} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

This PR deletes the 'WHT Business Posting Group', 'WHT Product Posting Group', 'WHT Absorb Base', 'Skip WHT', 'Actual Vendor No.', 'Is WHT', and related WHT fields outright from base APAC tables that hold existing customer data (G/L Account, Gen.

Journal Line, Posted Gen. Journal Line, Invoice Posting Buffer, Purchase Header/Line, Purch. Inv. Header/Line, Purch. Cr. Memo Hdr./Line, Purch. Rcpt. Header, Return Shipment Header, Purchase Header Archive, Vendor, Vendor Templ.) instead of staging the retirement with ObsoleteState = Pending (with ObsoleteReason and ObsoleteTag) and following up with a Removed stage plus an upgrade procedure. The fields are re-declared with the same field IDs via new table extensions, which preserves data only for tenants where the base-table deletion and the extension addition are published together in the exact same deployment. For most of the affected tables the replacement extension is co-located in the same APAC layer (e.g. WHTPurchaseLine.TableExt.al, WHTVendor.TableExt.al), so data continuity is likely intact there. However, for G/L Account, Gen. Journal Line, Posted Gen. Journal Line, and Invoice Posting Buffer the replacement fields were placed in an AU-only extension (src/Layers/AU/BaseApp/Local/WHT/*.TableExt.al) while the field was deleted from the shared APAC layer. Any tenant on a non-AU APAC-based country (for example NZ, whose layer config extends APAC directly without AU) will simply lose these fields and their stored WHT configuration permanently, with no migration path.

Recommendation:

  • stage the removal through ObsoleteState Pending -> Removed with ObsoleteReason/ObsoleteTag across releases, and if the field must move across a layer boundary that not every consumer of the source layer includes, ship an upgrade codeunit that migrates the data (or keep the field available at the APAC level, not AU-only) so non-AU APAC tenants are not silently dropped.

Knowledge:

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

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

@github-actions github-actions Bot removed the needs-approval Workflow runs require maintainer approval to start label Jul 8, 2026
[IntegrationEvent(true, false)]
[Scope('OnPrem')]
procedure OnGenJnlLineGetVendorAccount(Vendor: Record Vendor)
procedure OnGenJnlLineGetVendorAccount(Vendor: Record Vendor; var Rec: Record "Gen. Journal Line")

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\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 3}}$

OnGenJnlLineGetVendorAccount is an existing published [IntegrationEvent] (Scope('OnPrem')) whose signature is changed from a single 'Vendor: Record Vendor' parameter to '(Vendor: Record Vendor; var Rec: Record "Gen.

Journal Line")'. Widening a shipped publisher's signature is a breaking change for the extensibility contract: any existing extension already subscribing against the original one-parameter contract may not receive or expect the new mutable 'Rec' argument, and the semantics of 'Skip WHT' now being set via this event are invisible to callers written against the old contract. This is a cross-cutting API-compatibility concern (touches upgrade/extensibility and architecture, not a single leaf domain), so it is capped to minor per agent-finding policy even though the underlying impact on shipped extensibility surface is normally major-to-blocker.

Recommendation:

  • add a new event instead of mutating the existing publisher's signature, or stage the change through ObsoleteState/Reason/Tag on the old event before removal.

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

{
}
key(Key15; "Document Type", "Document No.", "WHT Business Posting Group", "WHT Product Posting Group")
key(Key15; "Document Type", "Document No.")

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

Key15 on Purchase Line previously was ("Document Type", "Document No.", "WHT Business Posting Group", "WHT Product Posting Group").

The PR relocates the two WHT fields to the WHTPurchaseLine table extension but redefines Key15 as just ("Document Type", "Document No."), dropping the WHT fields from the key entirely rather than re-declaring an equivalent key in the extension. Any lookup or sort that filtered/sorted on the WHT posting groups now has no covering index and falls back to a wider read or in-memory sort, per the referenced guidance on matching keys to filter/sort requirements.

Knowledge:

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

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

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

Same pattern a third time: Key4 ("Document No.", "WHT Business Posting Group", "WHT Product Posting Group") is deleted from Purch.

Cr. Memo Line and the paired WHTPurchCrMemoLine table extension does not re-declare an equivalent key, dropping index coverage for the WHT posting-group fields.

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

tableextension 28008 WHTPurchCrMemoLine extends "Purch. Cr. Memo Line"
{
    fields
    {
        field(28040; "WHT Business Posting Group"; Code[20])
        {
            Caption = 'WHT Business Posting Group';
            DataClassification = CustomerContent;
            TableRelation = "WHT Business Posting Group";
        }
        field(28041; "WHT Product Posting Group"; Code[20])
        {
            Caption = 'WHT Product Posting Group';
            DataClassification = CustomerContent;
            TableRelation = "WHT Product Posting Group";
        }
        field(28042; "WHT Absorb Base"; Decimal)
        {
            AutoFormatType = 1;
            AutoFormatExpression = GetCurrencyCode();
            Caption = 'WHT Absorb Base';
            DataClassification = CustomerContent;
        }
    }
    keys
    {
        key(WHTKey4; "Document No.", "WHT Business Posting Group", "WHT Product Posting Group")
        {
        }
    }
}

Knowledge:

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

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

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

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

The same pattern repeats here: Key5 ("Document No.", "WHT Business Posting Group", "WHT Product Posting Group") is deleted from Purch.

Inv. Line and the moved WHT fields are not covered by any key in the paired WHTPurchInvLine table extension, losing the index for any access filtering/sorting by these posting groups.

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

tableextension 28010 WHTPurchInvLine extends "Purch. Inv. Line"
{
    fields
    {
        field(28040; "WHT Business Posting Group"; Code[20])
        {
            Caption = 'WHT Business Posting Group';
            DataClassification = CustomerContent;
            TableRelation = "WHT Business Posting Group";
        }
        field(28041; "WHT Product Posting Group"; Code[20])
        {
            Caption = 'WHT Product Posting Group';
            DataClassification = CustomerContent;
            TableRelation = "WHT Product Posting Group";
        }
        field(28042; "WHT Absorb Base"; Decimal)
        {
            AutoFormatType = 1;
            AutoFormatExpression = '';
            Caption = 'WHT Absorb Base';
            DataClassification = CustomerContent;
        }
    }
    keys
    {
        key(WHTKey5; "Document No.", "WHT Business Posting Group", "WHT Product Posting Group")
        {
        }
    }
}

Knowledge:

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

👍 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