Skip to content

Fix #11390: Use ISO country codes in FatturaPA export - #11662

Open
Marco Antonio Mauro (marcus905) wants to merge 19 commits into
microsoft:mainfrom
marcus905:fix-11390-fatturapa-iso-country-code
Open

Marco Antonio Mauro (marcus905) wants to merge 19 commits into
microsoft:mainfrom
marcus905:fix-11390-fatturapa-iso-country-code

Conversation

@marcus905

@marcus905 Marco Antonio Mauro (marcus905) commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

What & why

The Italian FatturaPA exporter currently writes Business Central Country/Region Code values directly into FatturaPA elements such as IdPaese and Nazione.

However, Country/Region Code is the primary key of the Business Central Country/Region record and is not required to be the ISO 3166-1 alpha-2 code.

For example, a tenant can validly configure:

Country/Region Code = USA
ISO Code = US

The current exporter can therefore emit:

<IdPaese>USA</IdPaese>

instead of the FatturaPA-compatible ISO code:

<IdPaese>US</IdPaese>

This change resolves FatturaPA country values through the related Country/Region record and uses its ISO Code instead of assuming that the Business Central primary key is already the required ISO value.

The change:

  • uses Country/Region."ISO Code" when generating FatturaPA country-code elements;
  • applies the same resolution consistently to company, customer, tax representative, and transmission intermediary country values used by the FatturaPA exporter;
  • validates that referenced Country/Region records contain a two-character ISO code before export;
  • reports missing or invalid ISO-code setup through the normal FatturaPA error-message validation flow;
  • preserves the Business Central Country/Region Code primary key and existing address data without requiring tenants to rename their country records.

Linked work

Fixes #11390
Fixes AB#656997

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

Added regression coverage in FatturaPATest.Codeunit.al for the following scenarios:

  • a foreign Country/Region whose Business Central key differs from its ISO code is exported using the two-character ISO Code;
  • the ISO code is used for both IdPaese and Nazione;
  • a Country/Region without a valid two-character ISO code is detected during FatturaPA validation and cannot generate an electronic document.

Existing foreign-customer FatturaPA tests were also updated so their generated Country/Region test data contains a valid ISO code.

The localized Italian Base Application and test applications build successfully.

FatturaPA Test codeunit 144200 was executed locally:

Risk & compatibility

No standard field or table schema is changed.

Country/Region Code remains the Business Central primary key and is not required to be changed to an ISO value. FatturaPA serialization now resolves the corresponding Country/Region."ISO Code" when a two-character country code is required.

Existing tenants that already use ISO alpha-2 values as their Country/Region keys continue to produce the same XML as long as the corresponding ISO Code is configured consistently.

Country/Region records used by FatturaPA must now contain a valid two-character ISO Code. Missing or invalid setup is reported before XML generation instead of allowing an invalid country code to be serialized.

@github-actions github-actions Bot added From Fork Pull request is coming from a fork Team: Finance GitHub request for Finance area needs-approval Workflow runs require maintainer approval to start labels Sep 21, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept with Suggestions

What this PR does

This change resolves FatturaPA country values through the related Country/Region record and writes its two-character ISO Code. It also reports missing or invalid ISO setup before XML generation.

The seven changed XML values address the reported key-versus-ISO mismatch, and the customer regression test exercises the main scenario. One related domestic-country decision still compares record keys, so equivalent records with the same ISO code can still produce foreign-recipient XML behavior.

Problem-solution fit

Fit: Partial

The main serialization defect is fixed consistently across the identified XML values. The remaining key-based domestic-country comparisons and incomplete coverage leave related cases unresolved.

Suggestions

S1 (🟠 Moderate): Use ISO codes for domestic-country decisions
Compare the company and customer ISO codes when deciding whether to emit CodiceFiscale and a domestic CAP. Different Country/Region keys can represent the same ISO country, so the current comparisons can wrongly treat a domestic customer as foreign.

S2 (🟠 Moderate): Update self-billing vendor test setup
Give the mock Fattura vendor country a valid ISO Code and assert that its ISO value is exported. The existing self-billing export tests otherwise fail before reaching their assertions.

S3 (🟠 Moderate): Cover the other changed country sources
The new output test proves only the customer IdPaese and Nazione values. Add mismatched-key checks for company, tax representative, and transmission intermediary because those are separate changed branches.

Risk assessment and necessity

Risk: The production change is narrow and does not change schemas or public APIs. The main regression surface is FatturaPA and self-billing export when Country/Region keys differ from ISO codes or test data has no ISO Code.

Necessity: The change is required because configurable Business Central record keys can produce country values that FatturaPA does not accept. Resolving the ISO Code is the correct approach and avoids requiring tenants to rename existing country records.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11662 round=1 by=alexei-dobriansky at=2026-09-21T13:37:28.5893004Z lastSha=4cde42fc32c25ffe8b3c836ab69d07c76c033144 reviewKey=2cf433f233465eae35792b4e2f6b664cb15058b655684efff2aee12411452b7b suggestions=S1@9e6c5baa,S2@e05a296d,S3@2c4881c9

@github-actions github-actions Bot added needs-approval Workflow runs require maintainer approval to start and removed needs-approval Workflow runs require maintainer approval to start labels Sep 21, 2026
@github-actions github-actions Bot added needs-approval Workflow runs require maintainer approval to start and removed needs-approval Workflow runs require maintainer approval to start labels Sep 25, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Request Changes

What this PR does

This round changes domestic-country decisions to compare ISO codes, updates self-billing test setup, and adds coverage for company, tax representative, and transmission intermediary sources. These changes address the three previous suggestions.

The normal invoice path now validates the configured country sources before export. The self-billing vendor path still reaches the shared getter without the same length validation, so a nonblank one-character ISO value can be serialized.

Status of previous suggestions
ID Title Status Author response
S1 Use ISO codes for domestic-country decisions Addressed Fixed in ccc0b084.
S2 Update self-billing vendor test setup Addressed Fixed in af665823.
S3 Cover the other changed country sources Addressed Fixed in ccc0b084.
New observations (commits since round 1)

S4 (🔴 High): Validate ISO length in the shared getter
GetISOCountryCode only rejects a blank value. Require exactly two characters here so self-billing cannot create an invalid country value and then mark the VAT entry as exported.

Risk assessment and necessity

Risk: The main invoice path is now consistent and covered. The remaining risk is limited to malformed Country/Region data that bypassed field validation, but self-billing can then generate an invalid FatturaPA file and persist that the VAT entry was exported.

Necessity: Resolving configurable Country/Region keys to ISO alpha-2 values is required for valid FatturaPA output. Enforcing the two-character invariant in every export path is needed to complete the fix.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11662 round=2 by=alexei-dobriansky at=2026-10-01T20:13:02.7284787Z lastSha=72a0b51a4d270481029dda5975c2d01d305116a5 reviewKey=9ac5850709fca9c6750a730147e57c1346d724eb19bd2c2c232c5e28d862c6f2 suggestions=S1@9e6c5baa:addressed,S2@e05a296d:addressed,S3@2c4881c9:addressed,S4@57d18bfe:new parentRound=1

@marcus905

Copy link
Copy Markdown
Contributor Author

Thanks for the follow-up review. S4 is now addressed.

GetISOCountryCode() now enforces the two-character ISO invariant directly in the shared helper, so malformed nonblank ISO values cannot bypass validation in paths such as self-billing.

I also added a regression test with a one-character ISO value inserted without field validation to verify that the shared getter rejects it with the existing FatturaPA-specific validation error.

FatturaPA Test codeunit 144200 was rerun locally: 50 tests total, 42 passed, with the remaining 8 failures unchanged and limited to the existing Library - Error Message.LoadErrorMessages TestPage field-ID issue.

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 3

Recommendation: Accept

What this PR does

This round makes the shared FatturaPA country helper require an ISO code of exactly two characters and adds a regression test for malformed nonblank data.

The check now covers every caller of the shared helper, including self-billing. An invalid value stops XML generation before the VAT entry can be marked as exported, while the normal invoice validation continues to report the setup error before generation.

Status of previous suggestions
ID Title Status Author response
S1 Use ISO codes for domestic-country decisions Addressed Confirmed in the previous round.
S2 Update self-billing vendor test setup Addressed Confirmed in the previous round.
S3 Cover the other changed country sources Addressed Confirmed in the previous round.
S4 Validate ISO length in the shared getter Addressed The shared helper now requires exactly two characters, with regression coverage for malformed data.
New observations (commits since round 2)

None - the changes address S4 without introducing a new issue.

Risk assessment and necessity

Risk: The change is narrow, uses an established two-character ISO validation pattern, and changes no schema, public API, or event contract. The main invoice and self-billing paths now reject malformed country data before producing an invalid FatturaPA document.

Necessity: The validation completes the ISO-country fix and prevents invalid electronic documents when stored setup data bypasses normal field validation.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11662 round=3 by=alexei-dobriansky at=2026-10-02T07:48:11.3134417Z lastSha=07fb4f0cc0d4ba5e7bfdc4cb981dd428270793aa reviewKey=a9996ef2c49444a210df533fd4cfef6ea02ec68d9e6c17fea2f76492cedcef86 suggestions=S1@9e6c5baa:addressed,S2@e05a296d:addressed,S3@2c4881c9:addressed,S4@57d18bfe:addressed parentRound=2

Initialize();

// [GIVEN] A Country/Region with a one-character ISO Code inserted without field validation
CountryRegion.Init();

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\ —\ Testing}$

The test hand-rolls a Country/Region fixture with Init, an invented key, and Insert instead of using Library - ERM. Build the record through LibraryERM.CreateCountryRegion, then deliberately assign the malformed ISO value so the test retains its negative-case intent while relying on the maintained fixture setup.

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

        LibraryERM.CreateCountryRegion(CountryRegion);
        CountryRegion."ISO Code" := 'U';
        CountryRegion.Modify();

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.51.6

Initialize();

// [GIVEN] A foreign Country/Region without an ISO Code
CountryRegion.Init();

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\ —\ Testing}$

The test hand-rolls a Country/Region fixture with Init, an invented key, and Insert instead of using Library - ERM. Use LibraryERM.CreateCountryRegion so the prerequisite remains valid as the Country/Region schema evolves; the library-created record can still exercise the missing-ISO scenario.

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

        LibraryERM.CreateCountryRegion(CountryRegion);

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.51.6

@marcus905

Copy link
Copy Markdown
Contributor Author

Thanks for the additional review!

Both testing findings have now been addressed.

The affected tests use LibraryERM.CreateCountryRegion() instead of manually initializing and inserting Country/Region records.

The negative scenarios remain unchanged:

  • The shared helper test deliberately assigns a one-character ISO Code (U).
  • The missing-ISO export test explicitly clears the ISO Code.

I rebuilt and reran FatturaPA Test (codeunit 144200): 50 tests total, 42 passed, 8 known baseline failures.

Both affected regression tests passed. The eight unchanged failures are caused by the existing Library - Error Message.LoadErrorMessages TestPage field-ID issue (2074815070), unrelated to this PR.

No production code was modified in this follow-up.

Thanks again!

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 4

Recommendation: Accept

What this PR does

The FatturaPA exporter resolves Country/Region keys to two-character ISO codes. This round only replaces manual Country/Region creation in two negative tests with LibraryERM.CreateCountryRegion; production code is unchanged.

Both tests still assign their invalid values after creating the record: one uses U, and the other clears the ISO Code. This preserves the shared-getter and export-validation failure paths while using the standard test fixture.

Status of previous suggestions
ID Title Status Author response
S1 Use ISO codes for domestic-country decisions Addressed Both domestic-country decisions still compare resolved ISO codes.
S2 Update self-billing vendor test setup Addressed The vendor fixture still sets a valid ISO Code, and export assertions use ISO values.
S3 Cover the other changed country sources Addressed Mismatched-key checks remain for company, tax representative, and transmission intermediary country values.
S4 Validate ISO length in the shared getter Addressed The getter still requires exactly two characters, and the negative test retains its one-character value.
New observations (commits since round 3)

None - the fixture changes preserve the intended invalid-ISO scenarios.

Risk assessment and necessity

Risk: The latest commit changes test setup only. The production ISO lookup and validation are unchanged, with no new schema or event-contract changes. This was a static review; no build or tests were run.

Necessity: The ISO-country fix remains necessary because configurable Country/Region keys can otherwise produce invalid electronic invoices. Using the standard fixtures keeps the regression tests consistent with the test library without weakening their checks.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11662 round=4 by=alexei-dobriansky at=2026-10-27T11:28:50.9550692Z lastSha=f2cca5a347dd3b109e0168a04a1ca931cb67d327 reviewKey=d8951aaf0cc9581d4d91b890186764a36696cbe719122f29a134d4f46521730a suggestions=S1@9e6c5baa:addressed,S2@e05a296d:addressed,S3@2c4881c9:addressed,S4@57d18bfe:addressed parentRound=3

"No.", CreateAndPostSalesInvoice(DocumentRecRef, CreatePaymentMethod(), CreatePaymentTerms(), CustomerNo));

// [WHEN] The document is exported to FatturaPA
asserterror ElectronicDocumentFormat.SendElectronically(

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\ —\ Testing}$

The new negative test uses asserterror without asserting the error raised by the export call. Although the page handler checks that the expected validation message is displayed, an unrelated error can still satisfy asserterror; add an assertion for the outer error (or use the error-message library to assert the logged error) so the test cannot pass for the wrong failure.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.51.6

@marcus905

Copy link
Copy Markdown
Contributor Author

Thanks for the additional review!

The latest testing finding has now been addressed.

The negative ISO validation test now verifies both:

  • The specific FatturaPA ISO country validation message, through the existing error-message page handler.
  • The expected rollback error from SendElectronically, using Assert.AreEqual('', GetLastErrorText(), ...).

I verified the behavior against the standard Error Message.ShowErrorMessages(true) implementation, which displays the validation messages and raises Error('') to roll back the operation.

I rebuilt and reran FatturaPA Test (codeunit 144200): 50 tests total, 42 passed, 8 unchanged baseline failures. The corrected test passed.

No production code was modified.

Thanks!

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 5

Recommendation: Accept

What this PR does

The FatturaPA exporter resolves configurable Country/Region keys to two-character ISO codes and validates the country setup. This round only adds an assertion for the expected empty rollback error in the missing-ISO export test; production code is unchanged.

This round adds an exact empty-error assertion after the expected SendElectronically failure. The existing page handler checks the specific ISO validation message, while ShowErrorMessages(true) raises Error('') for the rollback, so the assertions cover different parts of the failure contract. Production code is unchanged in this round and all four previous fixes remain present.

Problem-solution fit

Fit: Strong

Resolving the separate ISO Code addresses the key-versus-country mismatch without renaming country records. The latest assertion accurately checks the existing validation rollback contract and strengthens the negative export regression test without changing its scenario.

Status of previous suggestions
ID Title Status Author response / evidence
S1 Use ISO codes for domestic-country decisions Addressed Both domestic-country decisions compare resolved ISO codes, and the matching-ISO regression test verifies the fiscal code and domestic post code for different country keys.
S2 Update self-billing vendor test setup Addressed The self-billing vendor fixture assigns ISO Code DE, and the vendor export assertions read the related ISO value rather than the country key.
S3 Cover the other changed country sources Addressed The header-country regression test uses separate country records for the company, tax representative, and transmission intermediary and asserts their ISO values in the generated XML.
S4 Validate ISO length in the shared getter Addressed The shared getter rejects every ISO value whose length is not two, and the malformed nonblank regression test checks the specific validation error. Self-billing reaches this getter during XML generation before the VAT entry is marked exported.
New observations (commits since round 4)

None - no new issues found in the changes since the previous round.

Risk assessment and necessity

Risk: The latest delta changes one negative test only, with no production, schema, API, event, or posting change. The overall ISO-country serialization and domestic-country checks remain the relevant regression surface; runtime execution was not independently reproduced.

Necessity: Without the ISO lookup, legitimate country keys that differ from ISO alpha-2 values can still produce rejected electronic invoices; renaming those keys is a disruptive workaround. The added assertion is useful because an unqualified expected error can otherwise conceal an unrelated export or handler failure.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11662 round=5 by=alexei-dobriansky at=2026-10-28T20:24:15.9650748Z lastSha=5c505a76227b76546bfde85828f9cc5f3a502c0c reviewKey=0d6be92ec628a14bc895b98e22fc3ebda244984987db2e1316cc38487e510e0b suggestions=S1@9e6c5baa:addressed,S2@e05a296d:addressed,S3@2c4881c9:addressed,S4@57d18bfe:addressed parentRound=4

Resolve the transmitter IdPaese/IdCodice overlap by retaining the
upstream GetCompanyTransmitterId calculation and the PR's ISO-based
country serialization. Preserve subsequent main changes, including
the current XSD test path configuration.

Assert the standard empty Dialog rollback explicitly in the missing
ISO country negative export test. The error-message page handler keeps
verifying the exact FatturaPA validation diagnostic.
@marcus905

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Addressed in 4636edd.

The negative ISO-country export test now asserts both Assert.ExpectedError('') and Assert.ExpectedErrorCode('Dialog'), matching the standard ShowErrorMessages(true) rollback behavior. The existing page handler independently verifies the specific FatturaPA ISO-country validation message.

I also merged the current upstream main and resolved the conflict in Export FatturaPA Document, preserving both the upstream GetCompanyTransmitterId implementation and our ISO-based country serialization.

Validation: Codeunit 144200 executed 50 tests; 42 passed, including all five ISO-country regression scenarios. The 8 remaining failures are unchanged test-environment issues (Error Messages TestPage failures).

The PR is now conflict-free and ready for renewed CI and review.

This branch was previously deployed

1 inactive (outdated) deployment
triage — 4cde42fc Deployed Sep 21, 2026 by marcus905 via Classify team ownership #5332
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ADO: Sync Creates an Azure DevOps work item if needed and indicates that one is linked From Fork Pull request is coming from a fork needs-approval Workflow runs require maintainer approval to start Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Italian FatturaPA exporter uses Business Central Country/Region Code instead of ISO alpha-2 country code

3 participants