[Prangshuman Das (t-prda)]

Review focus: LibraryGraphDocumentTools.EnsureVATPostingSetupExists and its call sites.

Area fix (previously #11224): ensures a VAT posting setup exists for API document fixtures, then re-enables the affected tests.

Related: AB#646383, AB#653393

Stack (each PR shows only its own layer; merge bottom first)

1. [API tests] Default to web service key authentication #12674 A: web service key auth (base: main)
2. [Expense tests] Re-enable Expense Agent API tests #12675 B: Expense API tests + clean test codeunit execution
3. [API tests] Re-enable APIV1 and APIV2 API tests #12676 C: re-enable APIV1/APIV2 API tests
4. [API tests] Simplify API test web service key handling #12890 Review fixes for A: simplify web service key handling
5. Area fixes: [API tests] Ensure VAT posting setup in API document fixtures #12677 D1 VAT fixtures → [API tests] Ensure cancellation reason codes in API sales document tests #12678 D2 cancellation reasons → [API tests] Handle localized purchase invoice payment discounts in APIV1 tests #12679 D3 localized discounts → [API tests] Set sales quote document and posting dates in API tests #12680 D4 quote dates → [API tests] Assign a number series to API vendor payment journals #12681 D5 journal numbering → [API tests] Ignore localized fields in API document comparisons #12682 D6 field comparison → [API tests] Return shipment PDF selection and pdfDocument test isolation #12683 D7 PDF documents → [API tests] Improve APIV1 fixture and verification reliability #12684 D8 APIV1 reliability

Experiments (on D8, do not merge): #12685 E single tenant, #12686 F no clean execution. Combined validation (do not merge): #12687, same tree as D8.

Platform: main already pins BCPlatform 30.0.56112.0, which includes the SQL connection-pool fix (AB#653457).

Note for CI: AL-Go's new-warning check needs a CI/CD baseline from the base branch, and features/* bases have none, so a PR build there fails with a 404. The current CI runs were triggered while these PRs targeted main, so their results are valid for the cumulative change.

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing}$

The newly enabled TestDeleteInUse negative test uses asserterror without checking the expected error message or code, so an unrelated setup or permission error can make the test pass without proving that an item variant cannot be deleted while in use.

Knowledge:

• microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing}$

The newly enabled TestDeleteInUse negative test uses asserterror without checking the expected error message or code, so an unrelated setup or permission error can make the test pass without proving that an item variant cannot be deleted while in use.

Knowledge:

• microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing}$

The re-enabled negative test uses asserterror without checking the expected error message or code, so an unrelated failure can make TestDeleteInUse pass without proving that deletion of an in-use variant is rejected. Add a stable ExpectedError, ExpectedErrorCode, or purpose-built assertion for the intended failure.

Knowledge:

• microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing}$

The re-enabled negative test uses asserterror without checking the expected error message or code, so an unrelated failure can make TestDeleteInUse pass without proving that deletion of an in-use variant is rejected. Add a stable ExpectedError, ExpectedErrorCode, or purpose-built assertion for the intended failure.

Knowledge:

• microsoft/knowledge/testing/asserterror-needs-expectederror-and-code.md

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

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

[alexei-dobriansky]

Good Sense Reviewer - Round 2

Recommendation: Accept with Suggestions

What this PR does

The current net change adds a shared test helper that creates a missing VAT Posting Setup combination, calls it before API document-line validation, and re-enables the affected APIV1 and APIV2 tests. The helper keeps existing setup unchanged and the inspected sales and purchase call sites pass the matching VAT business and product groups, so the fixture fix is narrow and consistent.

Status of previous suggestions

None - round 1 had no suggestions.

New observations (commits since round 1)

S1 (🟠 Moderate): Verify the intended item-variant delete failure
The removed exclusions re-enable both TestDeleteInUse variants, but each test accepts any error from the DELETE request. Add an expected HTTP status or stable error assertion so authentication, server, or setup errors cannot make these tests pass.

Risk assessment and necessity

Risk: Low production risk because the change is limited to test libraries, API tests, and disabled-test manifests. The remaining risk is a false-positive result in the re-enabled item-variant negative tests; no production API, event, report layout, or runtime behavior changes.

Necessity: The change provides the missing VAT setup combinations needed by API document fixtures and restores twelve disabled tests. Without it, useful API coverage remains disabled even though the document behavior is valid.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=12677 round=2 by=alexei-dobriansky at=2026-10-14T22:51:06.5849143Z lastSha=07f574dbfbf22d3a49498932410aa7af97b88388 reviewKey=fab4b7776a6fba17e88ba6ea51df84dcab6a4951c08ecbaaaa5c2f88ad66dc16 suggestions=S1@857c7b01:new parentRound=1