Extract IT withholding tax code

[Alexander-Ya]

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

[github-actions]

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]

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
Other	6	6	0	4	0

Totals: 6 knowledge-backed · 0 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]

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

The obsoleted procedure 'CheckWithholdTax' in Gen.

Jnl.-Post Line is guarded by '#if not CLREAN29' — the compiler symbol is misspelled ('CLREAN29' instead of 'CLEAN29'). Every other obsolete-removal block introduced in this same PR (in this file and in PurchPost.Codeunit.al) correctly uses '#if not CLEAN29'. Because 'CLREAN29' is never a real build symbol, 'not CLREAN29' is always true, so this block will never be compiled out — not even in a CLEAN29 build — defeating the staged-removal mechanic the referenced guidance describes ('the standard mechanic for retiring the actual implementation body is to remove the #if not CLEAN block in the same release that flips the state to Removed'). The obsolete wrapper will silently survive indefinitely alongside its now-duplicated logic in 'WHT Gen. Jnl.-Post Line IT'.

Suggested change

	#if not CLREAN29
	#if not CLEAN29

Knowledge:

• microsoft/knowledge/upgrade/obsolete-pending-to-removed-staging.md

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

[github-actions]

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

Two role-center tooltips in this page extension contain mojibake (workers´Š¢ compensation) instead of readable text, so users will see corrupted UI copy.

Replace the corrupted characters in both tooltips with a normal apostrophe or typographic apostrophe (workers' compensation / workers' compensation).

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

[github-actions]

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

The new Gen.

Journal Line subscribers for Account No. and Document Date immediately delegate to DeleteTmpWithhSocSec/UpdateTmpWithholdingContribution, which read the persistent Tmp Withholding Contribution table on every validation. This matches the anti-pattern of doing database work in frequently-fired subscribers without a cheap in-memory guard; add a fast exit so only lines that can actually have withholding state touch the table.

Knowledge:

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

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

[github-actions]

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

Label Text12100 has two placeholders (%1, %2) but no Comment explaining them.

Translators cannot infer from the text alone that %1 is the entry number and %2 is the document type, so the label should document both placeholders explicitly.

Suggested change

	Text12100: Label 'You cannot create the withhold entry from entry %1 because it''s an %2 Document.';
	Text12100: Label 'You cannot create the withhold entry from entry %1 because it''s an %2 Document.', Comment = '%1 = Entry No., %2 = Document Type';

Knowledge:

• microsoft/knowledge/style/label-comment-explains-placeholders.md

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

[github-actions]

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

This PR deletes WHT-backed fields directly from existing base tables and reintroduces them in tableextensions (for example Vendor, Vendor Templ., and Gen.

Journal Line) with no Pending deprecation window. That is an immediate contract break for dependent extensions that still compile against the base-table symbols; per the referenced guidance, these fields need a staged Pending → Removed lifecycle instead of disappearing in one release. Notably, other parts of this same PR (e.g. procedures in GenJnlPostLine.Codeunit.al and PurchPost.Codeunit.al) correctly wrap the moved logic in #if not CLEAN29 with [Obsolete(...)] attributes, showing the staged-removal convention was known but not applied consistently to the table fields themselves.

Knowledge:

• microsoft/knowledge/upgrade/obsolete-pending-to-removed-staging.md
• microsoft/knowledge/upgrade/obsoletion-requires-reason-and-tag.md

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

The move also removes public base-object entry points outright (OnBeforeValidateFiscalCode on Vendor and procedures such as DeleteTmpWithhSocSec / InitFields on Gen.

Journal Line) instead of first marking them obsolete with ObsoleteState, ObsoleteReason, and ObsoleteTag. Downstream extensions get no deprecation guidance and no versioned transition path before these symbols disappear.

Knowledge:

• microsoft/knowledge/upgrade/obsoletion-requires-reason-and-tag.md
• microsoft/knowledge/upgrade/obsolete-pending-to-removed-staging.md

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

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

[github-actions]

$\textbf{🔴\ Critical\ Severity\ —\ Other} \quad \color{gray}{\texttt{\small Iteration\ 4}}$

This subscriber does not copy the publisher signature exactly.

OnPrepareTempVendLedgEntryOnBeforeCheckAgainstApplnCurrency is published with (GenJournalLine, NewCVLedgerEntryBuffer, OldVendorLedgerEntry), but the subscriber skips the middle parameter and makes its second parameter OldVendorLedgerEntry. For event subscribers, the parameter list must match the publisher order; skipping a non-trailing parameter prevents the subscriber from binding/compiling.

Suggested change

	local procedure OnPrepareTempVendLedgEntryOnBeforeCheckAgainstApplnCurrency(GenJournalLine: Record "Gen. Journal Line"; OldVendorLedgerEntry: Record "Vendor Ledger Entry")
	local procedure OnPrepareTempVendLedgEntryOnBeforeCheckAgainstApplnCurrency(GenJournalLine: Record "Gen. Journal Line"; NewCVLedgerEntryBuffer: Record "CV Ledger Entry Buffer"; OldVendorLedgerEntry: Record "Vendor Ledger Entry")

Knowledge:

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Other} \quad \color{gray}{\texttt{\small Iteration\ 4}}$

Text1130023 is passed to Error() in this codeunit, but its name has no approved suffix.

Per AA0074, error labels should use an Err suffix so the call-site contract is visible from the identifier.

Knowledge:

• microsoft/knowledge/style/label-suffix-approved-list.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Other} \quad \color{gray}{\texttt{\small Iteration\ 4}}$

Text12100 is passed to Error() from this page extension, but its name has no approved suffix.

Per AA0074, error labels should use an Err suffix instead of a generic Text#### identifier.

Knowledge:

• microsoft/knowledge/style/label-suffix-approved-list.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Other} \quad \color{gray}{\texttt{\small Iteration\ 4}}$

This codeunit calls its own procedures as bare identifiers (CheckWithholdingTaxTotalAmount, PostWithhSocSec, GetCompWithhTaxTotalAmount) instead of this.<procedure>(...).

The this prefix is the BC style convention for explicit self-reference inside codeunits.

Knowledge:

• microsoft/knowledge/style/this-keyword-in-codeunits.md

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