[WHT]-Withholding Tax is not deducted from Expense Report lines based on the Expense Category during posting

[ViditGupta1277]

Fixes AB#645160

[github-actions]

Could not find linked issues in the pull request description. Please make sure the pull request description contains a line that contains 'Fixes #' followed by the issue number being fixed. Use that pattern for every issue you want to link.

[ViditGupta1277]

Copilot resolve the merge conflicts in this pull request

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Performance}$

The new OnAfterExpenseCategoryValidateEmployee subscriber calls ExpenseCategory.Get immediately on every validate of the Expense Category field, so clearing or re-entering the field still pays a database lookup unconditionally. Add a cheap blank-value guard before the Get and only hit the table when a category code is present.

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

        if CheckWithholdingTaxDisabled() then
            exit;

        if Rec."Expense Category" = '' then begin
            Rec."Wthldg. Tax Prod. Post. Group" := '';
            exit;
        end;

        if ExpenseCategory.Get(Rec."Expense Category") then
            Rec."Wthldg. Tax Prod. Post. Group" := ExpenseCategory."Wthldg. Tax Prod. Post. Group"
        else
            Rec."Wthldg. Tax Prod. Post. Group" := '';

Knowledge:

• microsoft/knowledge/performance/guard-event-subscribers-before-db-call.md

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Agent}$

Two independently-flagged issues compound: the buffer table is directly writable (RIMD) by any user holding only the new permission set, and PostWHTSingle/PostWHTGroup post whatever amounts are found in that buffer without validating that a posting setup exists before trusting the amount. Together, a user (or a bug) that seeds/edits buffer rows can drive incorrect G/L postings with no configuration error surfaced anywhere in the flow. Closing either gap alone still leaves the other; both the permission model and the setup validation should be hardened together.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🔴\ Critical\ Severity\ —\ AppSource}$

The new assignable permission set "Exp. Withholding Tax" grants only the buffer table and two internal codeunits, but the app's setup and normal usage paths also depend on the Expense Category Card, Withholding Tax Posting Setup, and the extended General Journal page/fields. Without execute/tabledata coverage for those objects (or an equivalent included permission set), an assigned user cannot configure or use this feature without SUPER.

Knowledge:

• microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

Table 7059 is modeled as a normal persisted table even though it is used only as a per-document staging buffer between posting events and is explicitly deleted again after posting. Keeping transient posting state in tabledata adds unnecessary schema and permission surface, and any path that misses cleanup leaves stale rows behind. Model this buffer as temporary, or document and constrain why durable storage is required.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

In PostWHTGroup, a missing component posting setup only skips G/L entry creation, but InsertEmployeeWHTComponentEntry still runs. Posting continues with withholding tax component history but no balancing payable entry, and the user never sees a configuration error. Fail the posting when the component setup row is absent before inserting the component tax entry.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Events}$

The OnAfterIsEmployeeWHTApplicable subscriber overwrites IsApplicable with true in its else branch, so it can undo another subscriber's decision to set it false, depending on execution order. Only narrow the result when this subscriber needs to block withholding tax, and otherwise leave the incoming value unchanged.

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

if (GenJnlLine."Wthldg. Tax Prod. Post. Group" = '') and IsExpenseCategorySingleTax(GenJnlLine) then
    IsApplicable := false;

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🔴\ Critical\ Severity\ —\ Style}$

New event subscriber OnAfterIsEmployeeWHTApplicable declares (GenJnlLine, var IsApplicable) but the publisher in WHT Employee Calculation raises the event as (GenJnlLine, Employee, var IsApplicable). AL subscriber parameter lists must be a positional prefix of the publisher's list — you may drop trailing parameters but not a middle one. Here the subscriber's second parameter (var IsApplicable: Boolean) lines up against the publisher's second parameter (Employee: Record Employee), a type mismatch that will fail compilation of this app. Add the missing Employee: Record Employee parameter between GenJnlLine and var IsApplicable.

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

    local procedure OnAfterIsEmployeeWHTApplicable(GenJnlLine: Record "Gen. Journal Line"; Employee: Record Employee; var IsApplicable: Boolean)

Knowledge:

• microsoft/knowledge/style/event-subscriber-param-names-match-publisher.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Agent}$

This PR introduces a second, parallel implementation of single/group withholding-tax GL posting logic. WHTExpReportPostHandler.PostWHTSingle/PostWHTGroup (new, ExpenseWithholdingTax app) closely mirrors the pre-existing WthldgTaxEmplSubscribers.PostEmployeeWithholdingTax/PostEmployeeWHTGroupEntries (WithholdingTax app): both resolve WHT group components, iterate Withholding Tax Group Line, compute compounded bases, and call GenJnlPostLine.InitGLEntry/InsertGLEntry with a negated tax amount against the payable WHT account. The two implementations already differ subtly (e.g. how the base/compounding accumulates and how the group-vs-single decision integrates with IsEmployeeWHTApplicable), which is a maintenance/architecture risk: a future fix to the withholding-tax calculation or posting rules would need to be applied in two places, and the two code paths can silently drift apart. This is a cross-cutting architecture concern spanning the (pre-existing) WithholdingTax posting domain and the new ExpenseWithholdingTax feature, not owned by any single leaf review domain; recommend extracting the shared single/group posting logic into one reusable procedure that both callers invoke.

Posting this finding as an issue comment because inline comment placement failed.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

Behavioral change to the shipped, non-local procedure IsEmployeeWHTApplicable in WHTEmployeeCalculation.Codeunit.al (WithholdingTax app, currently at version 29.0.0.0 with prior released history). Previously the procedure returned false outright whenever GenJnlLine."Wthldg. Tax Prod. Post. Group" was empty. That hard exit is now removed; IsApplicable defaults to true and is only overridden by the new OnAfterIsEmployeeWHTApplicable event (to which, elsewhere in this same PR, the sole subscriber has a broken signature — see the Events finding above, meaning the override may not even fire). For any journal line with no product posting group and no other applicable subscriber, the procedure's return value flips from false to true, so withholding tax may now be calculated where it previously was not. The procedure's external signature is unchanged, so this is a behavior contract change rather than a compile-time break, but it can silently alter WHT calculation outcomes for existing customers/extensions relying on the old behavior. Flagging as a candidate for a curated breaking-change rule on behavior-only contract changes to shipped procedures.

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4}