Features/494919 master mx cfdi rounding e invoice docs#9697
Conversation
…tests' of https://github.com/microsoft/BCApps into bugs/639734-master-Disable-mx-cdfi-date-boundary-flaky-tests
…r de igualdad exacta
…adas al fisco. Creacion de test relacionados al caso
| '0000C72', StrSubstNo(StampReqMsg, GetDocTypeTextFromDatabaseId(DocumentHeaderRecordRef.Number)), Verbosity::Normal, DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, 'Category', MXElectronicInvoicingTok); | ||
| CurrencyDecimalPlaces := GetCurrencyDecimalPlaces(TempDocumentHeader."Currency Code"); | ||
|
|
||
| // Recalculate SubTotal to match Σ(Importe) as written in XML (CFDI40108 compliance) |
There was a problem hiding this comment.
The new SubTotal-recalculation comment in RequestStamp contains a corrupted/mojibake byte sequence (“╬u”, hex E2 95 AC C3 BA) where a Sigma summation symbol was clearly intended (“Recalculate SubTotal to match Σ(Importe) as written in XML”). This indicates an encoding mishap when the comment was authored (e.g. a copy-paste through a lossy encoding round-trip) and leaves unreadable garbage characters permanently committed to the source file. It is cosmetic but should be fixed before merge since it will render as mangled text in every editor/diff viewer and could trip encoding-sensitive tooling.
| // Recalculate SubTotal to match Σ(Importe) as written in XML (CFDI40108 compliance) | |
| // Recalculate SubTotal to match Σ(Importe) as written in XML (CFDI40108 compliance) |
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| '0000C72', StrSubstNo(StampReqMsg, GetDocTypeTextFromDatabaseId(DocumentHeaderRecordRef.Number)), Verbosity::Normal, DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, 'Category', MXElectronicInvoicingTok); | ||
| CurrencyDecimalPlaces := GetCurrencyDecimalPlaces(TempDocumentHeader."Currency Code"); | ||
|
|
||
| // Recalculate SubTotal to match Σ(Importe) as written in XML (CFDI40108 compliance) |
There was a problem hiding this comment.
CreateTempDocument is a public procedure, and this change alters its observable outputs by recomputing SubTotal, TotalDiscount, TempDocumentHeader.Amount, and TempDocumentHeader."Amount Including VAT" for non-transfer, non-advance-settle documents. Any dependent code (including extensions) that already calls this existing public procedure directly can now receive different totals for the same input document than before this change. Consider keeping the existing procedure's returned values stable and moving the XML-specific rounding adjustment behind a new internal helper or overload used only by the stamping flow, or clearly documenting the behavior change if it is intentional and acceptable.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
|
The auto-generated reverse stamp issued for advance-settlement invoices still calls RequestStamp(...) directly instead of the new RequestStampWithRoundingFallback(...). That means the second CFDI produced for an advance-settlement flow never benefits from the new retry/rounding-model-selection logic and can still fail with CFDI40108/CFDI40167 even though ordinary (non-advance-settle) invoice stamping is fixed by this PR. This leaves a real production path that this PR was meant to cover still exposed to the original bug. Suggested fix (apply manually — could not be anchored as a one-click suggestion): if AdvanceSettle then
if SalesInvoiceHeader."Electronic Document Status" = SalesInvoiceHeader."Electronic Document Status"::"Stamp Received" then
RequestStampWithRoundingFallback(DocumentHeaderRecordRef, true, true);Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4 |
|
|
||
| // Recalculate SubTotal to match Σ(Importe) as written in XML (CFDI40108 compliance) | ||
| // Each line's Importe is rounded to 6 decimals; SubTotal must equal their sum. | ||
| // NOTE: Exclude AdvanceSettle because the advance-settle XML/original-string paths currently derive Total using |
There was a problem hiding this comment.
The new SubTotal/Total recalculation added to guarantee CFDI40108/CFDI40119 compliance is explicitly skipped when AdvanceSettle is true, but the advance-settle XML generation path (CreateXMLDocument33AdvanceSettle) still writes per-line Importe values from GetReportedLineAmount() while SubTotal continues to come from the old, non-recalculated aggregate. Prepayment-settlement sales invoices can therefore still hit the exact CFDI40108/CFDI40119 mismatch this PR is meant to eliminate for ordinary invoices.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| RequestStamp(DocumentHeaderRecordRef, Prepayment, Reverse); | ||
| StampAttempts += 1; | ||
| ErrorCode := DocumentHeaderRecordRef.Field(10035).Value(); | ||
| if not (ErrorCode in ['CFDI40108', 'CFDI40110', 'CFDI40111', 'CFDI40119', 'CFDI40167']) then begin |
There was a problem hiding this comment.
RequestStampWithRoundingFallback treats any PAC error code outside the hardcoded rounding-related list (CFDI40108/40110/40111/40119/40167) as success and exits immediately after the first RequestStamp call, without checking whether a later model attempt actually reached that state. If an alternative rounding model still fails but with a different, unrelated PAC validation code, the fallback logs RoundingFallbackSucceededMsg (a false-positive success telemetry event) and never tries the remaining rounding models, even though the document was not actually stamped successfully.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| end; | ||
| end; | ||
|
|
||
| Session.LogMessage('0000MF6', StrSubstNo(RoundingFallbackExhaustedMsg, StampAttempts, ErrorCode), Verbosity::Error, DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, 'Category', MXElectronicInvoicingTok); |
There was a problem hiding this comment.
The terminal rounding-fallback failure (event id 0000MF6, RoundingFallbackExhaustedMsg) is logged with TelemetryScope::ExtensionPublisher. This is a customer-actionable integration failure - stamping ultimately failed after exhausting every rounding model - so hiding it from environment telemetry means the tenant operator cannot alert or query on it. Emit it with TelemetryScope::All (or duplicate it there) while keeping the publisher-only diagnostic events (0000MF4/0000MF5) as-is.
| Session.LogMessage('0000MF6', StrSubstNo(RoundingFallbackExhaustedMsg, StampAttempts, ErrorCode), Verbosity::Error, DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, 'Category', MXElectronicInvoicingTok); | |
| Session.LogMessage('0000MF6', StrSubstNo(RoundingFallbackExhaustedMsg, StampAttempts, ErrorCode), Verbosity::Error, DataClassification::SystemMetadata, TelemetryScope::All, 'Category', MXElectronicInvoicingTok); |
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
|
|
||
| // [WHEN] RequestStampEDocument is called on Posted Sales Invoice "SI" | ||
| ErrorMessages.Trap(); | ||
| asserterror SalesInvoiceHeader.RequestStampEDocument(); |
There was a problem hiding this comment.
RequestStampRaisesNoRelationDocsErrorWhenCFDIRelation04 uses asserterror on SalesInvoiceHeader.RequestStampEDocument() but never pins the raised error with Assert.ExpectedError or Assert.ExpectedErrorCode. Only the Error Messages page assertion checks which validation fired; any later unrelated failure still satisfies the bare asserterror, so the test can pass while certifying the wrong failure path.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| PaymentNo); | ||
|
|
||
| // [THEN] The date portion of Comprobante/@Fecha equals the date portion of pago20:Pago/@FechaPago | ||
| // [THEN] Comprobante/@Fecha and pago20:Pago/@FechaPago are both within 1 day of the stamp date |
There was a problem hiding this comment.
The 'same posting date == same stamp date' payment-stamp test no longer checks that Comprobante/@Fecha and pago20:Pago/@FechaPago are equal to each other. It now only checks each is within +/-1 day of StampDate independently, so a regression where the two attributes serialize to different dates (e.g. one day apart) would still pass. This weakens the exact scenario the test name and original assertion were meant to protect; consider keeping a direct equality check between the two normalized dates in addition to the tolerance-based check against StampDate.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| CountryRegion.Insert(true); | ||
| Customer.Validate("Country/Region Code", CountryRegion.Code); | ||
| Customer.Address := LibraryUtility.GenerateGUID(); | ||
| if not PostCode.FindFirst() then begin |
There was a problem hiding this comment.
CreateSalesHeaderForStamp creates a new Country/Region but then satisfies the customer's postcode prerequisite with an unfiltered PostCode.FindFirst(), reusing whatever Post Code happens to exist first instead of one that belongs to the newly created country. That makes the customer's country/postcode combination depend on unrelated seed data and can make these tests flaky or environment-dependent.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| '0000C72', StrSubstNo(StampReqMsg, GetDocTypeTextFromDatabaseId(DocumentHeaderRecordRef.Number)), Verbosity::Normal, DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, 'Category', MXElectronicInvoicingTok); | ||
| CurrencyDecimalPlaces := GetCurrencyDecimalPlaces(TempDocumentHeader."Currency Code"); | ||
|
|
||
| // Recalculate SubTotal to match Σ(Importe) as written in XML (CFDI40108 compliance) |
There was a problem hiding this comment.
RequestStamp now recalculates SubTotal, TotalDiscount, TempDocumentHeader.Amount, and TempDocumentHeader."Amount Including VAT" before building the digital stamp XML, but the QR-code generation for sales invoices and sales credit memos (ProcessResponseESalesInvoice / ProcessResponseESalesCrMemo) still reads the posted header's original, unrecalculated "Amount Including VAT". When the rounding-fallback recalculation changes the stamped total by even one cent, the printed QR code encodes a total amount that no longer matches the PAC-signed CFDI, which can fail SAT/customer verification even though stamping itself succeeded. The QR code should be generated from the same stamped/recalculated total used for the XML sent to the PAC.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| FormatDate(Today()), | ||
| CopyStr(FechaValue, 1, 10), | ||
| 'Comprobante/@Fecha date portion must equal the stamp request date'); | ||
| AssertDateWithinOneDayTolerance( |
There was a problem hiding this comment.
AssertDateWithinOneDayTolerance replaces exact-date assertions with a +/-1 day tolerance in the payment-stamp date tests (Comprobante/@Fecha and pago20:Pago/@FechaPago), while no production date-handling code in this PR changed for those attributes. A one-day shift in either attribute - which would previously have failed the test - now passes silently. If the tolerance is compensating for a real, pre-existing timezone-conversion inconsistency between the two date paths, that inconsistency should be fixed or explicitly called out rather than absorbed by loosening the assertion.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| Commit(); | ||
| end; | ||
|
|
||
| local procedure SetupCFDIForRequestStamp() |
There was a problem hiding this comment.
SetupCFDIForRequestStamp enables "Sim. Request Stamp", which makes the PAC-invoking procedure exit before any simulated PAC response is processed. The new RequestStampSalesInv11LinesDoesNotRaiseCFDI40108, RequestStampFallbackToAlternativeRoundingModel, and RequestStampRetentionLinesExcludedFromSubTotal tests only ever observe the later, unrelated 'Root element is missing' XML-parsing failure, so they never actually exercise the new RequestStampWithRoundingFallback retry/model-selection logic against a real CFDI40108/CFDI40167 PAC response. The tests would still pass even if the fallback selection logic itself were broken.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
ISSUE
When requesting a stamp (Request Stamp) on sales invoices in the Mexican localization of Business Central, the generated CFDI XML produced rounding mismatches in SubTotal and per-line tax amounts, causing PAC validation errors:
CFDI40108 – SubTotal does not equal the sum of line Importe values
CFDI40167 – Per-line tax Importe does not equal
Round(Base × TasaOCuota, 6)
This occurred primarily when:
The invoice contained multiple lines (e.g., 11+)
Unit prices and quantities produced amounts where standard rounding introduced accumulated differences
Retention lines were present alongside regular lines
CAUSE
The logic responsible for calculating SubTotal in the CFDI XML did not recalculate the value as the sum of individually rounded line Importe values (rounded to 6 decimal places).
Instead, it used the aggregated amount directly, which could differ from the SAT-expected value (Σ Round(Importe, 0.000001)).
Additionally, the rounding fallback mechanism attempted models sequentially after PAC rejection (Model1 → Model2 → Model3 → Model4), which was inefficient and could still fail when no local pre-validation was performed to identify a valid rounding model before calling the PAC.
SOLUTION
Changes implemented in EInvoiceMgt.Codeunit.al:
SubTotal recalculation for CFDI40108 compliance
Added logic in RequestStamp to recalculate SubTotal as
Σ Round(Importe, 0.000001) for non-retention lines, ensuring consistency with the values written per line in the XML.
Total recalculation for CFDI40119 consistency
After recalculating SubTotal, Total is recomputed as:
SubTotal - Descuento + Trasladados - Retenidos.
Rounding model pre-validation (FindValidRoundingModel)
Performs local validation across available rounding models (0–3) before sending the document to the PAC, selecting a model that does not trigger CFDI40167 validation errors per line.
Unified fallback mechanism (RequestStampWithRoundingFallback)
Replaces the sequential call pattern with a single procedure that:
Local tax validation (ValidateDocumentAmountsLocal)
Validates that each line's tax Importe equals:
Round(Base × VAT% / 100, 0.000001)
State reset before stamp requests
Added Clear(EInvoiceCommunication) to prevent residual state between retries.
TESTS
Added MX AL tests in codeunit 144000 "MX CFDI Unit Test" to validate the fix and prevent regressions:
RequestStampSalesInv11LinesDoesNotRaiseCFDI40108
Validates that CFDI40108 is not raised during RequestStamp execution for a posted sales invoice with 11 lines (Qty = 1, Unit Prices 90–100, VAT 16%).
RequestStampFallbackToAlternativeRoundingModel
Validates that CFDI40167 is not raised during RequestStamp execution in scenarios with quantities that introduce rounding differences.
RequestStampRaisesNoRelationDocsErrorWhenCFDIRelation04
Regression coverage: validates that non-rounding errors (e.g., missing relation documents for CFDI Relation '04') are raised as expected.
RequestStampRetentionLinesExcludedFromSubTotal
Validates that CFDI40108 is not raised during RequestStamp execution when retention lines (Retention Attached to Line No. <> 0) are present.
SubTotalEqualsRoundedSumOfLineImportes
Unit test validating that CreateTempDocument produces a SubTotal equal to
Σ Round(Amount + Line Discount Amount, 0.000001) for all non-retention lines.
Fixes:
AB#638201
Slice 494919: [RegF][Vendor] MX CFDI: Rounding in electronic invoice documents.