[FR E-Reporting] Implement invoice payment lifecycle tracking - #9659
[FR E-Reporting] Implement invoice payment lifecycle tracking#9659Milica Đukić (djukicmilica) wants to merge 82 commits into
Conversation
GitHub Actions
Inactive
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Milica Đukić (djukicmilica)
added
Finance
GitHub Actions
Inactive
github-actions
Bot
added
Integration
GitHub Actions
Inactive
|
The pageextension declares a namespace but still adds unaffixed members to the base "E-Documents" page: field("Clearance Date"; ...) and action(ViewFREInvoiceLifecycles). A namespace only replaces the owned-object affix; members added to another publisher's page still need the registered app prefix or suffix, otherwise AppSourceCop AS0011 can reject the extension. Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.12.4 |
|
This Session.LogMessage emits an internal ErrorCallStack diagnostic with TelemetryScope::All, which exposes publisher-only implementation detail to environment telemetry. Per telemetry-scope guidance, this kind of low-level failure diagnostic should stay publisher-only; use ExtensionPublisher here (or split the tenant-facing failure signal from the publisher-only stack trace). Suggested fix (apply manually — could not be anchored as a one-click suggestion): Session.LogMessage(
'0000TDQ', LifecycleWorkerFailedTelemetryMsg, Verbosity::Error,
DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher,
'Category', LifecycleTelemetryCategoryTok, 'ErrorCallStack', GetLastErrorCallStack());Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4 |
|
Follow-up on the general review findings:
|
|
|
||
| enum 10971 "FR Regulatory Comment Type" | ||
| { | ||
| Extensible = true; |
There was a problem hiding this comment.
FR Regulatory Comment Type is modeled as Extensible = true, but the export code serializes the enum member name directly into the regulatory note code. That allows extensions to persist arbitrary values outside the standardized code list and still emit them in outbound XML. Make this enum non-extensible, or add validation that rejects nonstandard extension values before they can be stored/exported.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| begin | ||
| OriginalLifecycleVAT.SetRange("Lifecycle Entry No.", OriginalOccurrenceEntryNo); | ||
| if not OriginalLifecycleVAT.FindSet() then | ||
| Error(OriginalVATBreakdownErr, OriginalOccurrenceEntryNo); |
There was a problem hiding this comment.
This is an internal lifecycle-integrity failure, but it is raised as a plain client-visible error with a raw occurrence entry number. End users cannot correct a missing VAT breakdown on an already-captured lifecycle occurrence, so raise it as an ErrorInfo with ErrorType::Internal instead of exposing the technical detail directly.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
RaiseInternalError(StrSubstNo(OriginalVATBreakdownErr, OriginalOccurrenceEntryNo));Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| exit(TempBlob); | ||
|
|
||
| TempBlob.CreateInStream(PdfInStream); | ||
| if not TryGetEmbeddedAttachment(PdfInStream) then |
There was a problem hiding this comment.
ExtractCIIXml() treats every TryGetEmbeddedAttachment() failure as "The PDF file does not contain an embedded Factur-X invoice." Because TryGetEmbeddedAttachment() is a [TryFunction], malformed or unreadable PDFs are also collapsed into that same message, which hides the real parsing failure. Distinguish "no embedded invoice" from unexpected PDF parsing errors so corrupt PDFs are surfaced with a different error path.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
|
Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4 |
| if EventDate = 0D then | ||
| RaiseInternalError(EventDateErr); | ||
|
|
||
| case LifecycleStatus of |
There was a problem hiding this comment.
The new lifecycle suite never exercises the unsupported-status branch in CapturePaymentOccurrence, so the internal-error contract for a non-payment lifecycle status can regress unnoticed. Add a negative test that calls the new lifecycle management code with a status outside Collected/Negative Collected and asserts the expected internal error.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| VATEntry.SetLoadFields("VAT Bus. Posting Group", "VAT Prod. Posting Group", "Source Currency Code", "Source Currency VAT Base", "Source Currency VAT Amount", Base, Amount); | ||
| if VATEntry.FindSet() then | ||
| repeat | ||
| VATPostingSetup.Get(VATEntry."VAT Bus. Posting Group", VATEntry."VAT Prod. Posting Group"); | ||
| VATRate := VATPostingSetup."VAT %"; |
There was a problem hiding this comment.
CreateVATBreakdown reads only VAT Posting Setup."VAT %" inside the VAT Entry loop, but VATPostingSetup.Get(...) materializes the full setup row for every VAT entry because no SetLoadFields is applied to that record. Add VATPostingSetup.SetLoadFields("VAT %") before the loop so the hot path transfers only the field it uses.
| VATEntry.SetLoadFields("VAT Bus. Posting Group", "VAT Prod. Posting Group", "Source Currency Code", "Source Currency VAT Base", "Source Currency VAT Amount", Base, Amount); | |
| if VATEntry.FindSet() then | |
| repeat | |
| VATPostingSetup.Get(VATEntry."VAT Bus. Posting Group", VATEntry."VAT Prod. Posting Group"); | |
| VATRate := VATPostingSetup."VAT %"; | |
| VATEntry.SetLoadFields("VAT Bus. Posting Group", "VAT Prod. Posting Group", "Source Currency Code", "Source Currency VAT Base", "Source Currency VAT Amount", Base, Amount); | |
| VATPostingSetup.SetLoadFields("VAT %"); | |
| if VATEntry.FindSet() then | |
| repeat | |
| VATPostingSetup.Get(VATEntry."VAT Bus. Posting Group", VATEntry."VAT Prod. Posting Group"); | |
| VATRate := VATPostingSetup."VAT %"; |
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| ShipmentPostingDate: Date; | ||
| begin | ||
| LineXPath := StrSubstNo(InvoiceLineXPathTok, Format(SalesInvoiceLine."Line No.", 0, 9)); | ||
| if not XmlDoc.SelectSingleNode(LineXPath, NamespaceMgr, InvoiceLineNode) then |
There was a problem hiding this comment.
InjectExtendedCTCFranceElements iterates Sales Invoice Line records and, for each row, InjectExtendedLineReferences re-searches the full XML document with XmlDoc.SelectSingleNode(LineXPath, ...). That makes line-reference injection perform one root XPath traversal per invoice line. Select the invoice line nodes once and index them by cbc:ID before the loop, then reuse those nodes when adding order and shipment references.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| FREInvoiceLifecycle."Processing Status" := FREInvoiceLifecycle."Processing Status"::"Message Created"; | ||
| Clear(FREInvoiceLifecycle."Last Error"); | ||
| FREInvoiceLifecycle.Modify(); | ||
| Session.LogMessage( |
There was a problem hiding this comment.
The new lifecycle status traces in CreateLifecycleMessage and ScheduleMessageCreation log publisher-only background pipeline state to TelemetryScope::All. The created and queued events are internal implementation diagnostics rather than tenant-actionable failures, so emitting them to environment telemetry adds noise; use TelemetryScope::ExtensionPublisher for these status-transition events and keep TelemetryScope::All for actionable failures such as the worker error path.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| InherentEntitlements = X; | ||
| InherentPermissions = X; | ||
|
|
||
| procedure BuildMessage(EDocument: Record "E-Document"; ResponseType: Enum "E-Doc. Response Type"; var TempBlob: Codeunit "Temp Blob") |
There was a problem hiding this comment.
The added lifecycle tests never invoke "FR E-Invoice Lifecycle Msg." through its IEDocMessageBuilder.BuildMessage entrypoint; they all go through CreateLifecycleMessage, so the queued-occurrence lookup and the "No unprocessed French invoice lifecycle occurrence exists..." failure path in the interface method remain untested. Add a direct negative test for BuildMessage with no queued lifecycle record so regressions in the framework entrypoint cannot slip through.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
|
The new negative import tests verify Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4 |
| DataClassification = CustomerContent; | ||
| ToolTip = 'Specifies the name of the approved platform that sent the invoice.'; | ||
| } | ||
| field(21; "Invoice Issuer ID"; Text[50]) |
There was a problem hiding this comment.
The table stores "Invoice Issuer ID" as free-form Text[50] even though the lifecycle code always emits it under scheme 0002 (SIREN). In France, "Registration No." is a Text[20] field, so formatted values such as spaces or punctuation can be frozen into lifecycle rows and later sent as an invalid 0002 identifier. Constrain this field to a normalized SIREN shape (for example Code[9]) and populate it from normalized company data before inserting the lifecycle row.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| begin | ||
| Rec.TestField("Processing Status", Rec."Processing Status"::Queued); | ||
| Rec.TestField("E-Document Message Entry No.", 0); | ||
| Session.LogMessage( |
There was a problem hiding this comment.
The new French invoice lifecycle telemetry ships with placeholder-style Session.LogMessage event IDs (0000TDP, 0000TDQ, 0000TDR, 0000TDS, 0000TDT) across FREInvoiceLifecycleMsg, FREInvoiceLifecycleError, and FREInvoiceLifecycleMgt. Placeholder IDs are not stable catalogue IDs, so these events will be hard to query and can collide with other placeholder-based telemetry. Replace each with a registered, permanent event ID before merge.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
|
|
||
| // Use SIRET as endpoint with scheme 0009 | ||
| if CompanyInformation."SIRET No." = '' then | ||
| if not GetServiceParticipantAddress(EDocumentServiceCode, Enum::"E-Document Source Type"::Company, '', ElecAddress, ElecAddressScheme) then |
There was a problem hiding this comment.
The PEPPOL FR export now falls back to the company VAT registration number with scheme 9957 when both SIRET and Registration No. are blank, but the added tests only cover the SIRET, SIREN, and buyer-VAT fallback paths. Add a seller-path regression test that clears both company identifiers and verifies /Invoice/.../EndpointID uses the VAT number with scheme 9957, otherwise this new branch can regress unnoticed.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| /// Spec reference: https://docs.peppol.eu/poacc/billing/3.0/syntax/ubl-invoice/tree/ | ||
| /// </summary> | ||
| codeunit 10985 "E-Doc. Peppol BIS 3.0 FR Hdlr" implements IStructuredFormatReader | ||
| codeunit 10980 "E-Doc. Peppol BIS 3.0 FR Hdlr" implements IStructuredFormatReader |
There was a problem hiding this comment.
Codeunit "E-Doc. Peppol BIS 3.0 FR Hdlr" is renumbered from 10985 to 10980 in this PR (the vacated ID 10985 is immediately reused for the new "FR E-Invoice Lifecycle Error" codeunit) with no obsoletion window. Both codeunits are Access = Internal, so external partner extensions cannot reference either by symbol and cannot fail to compile from this change, which narrows the practical blast radius versus a public-object rename. However, any persisted data, telemetry, or tooling keyed on the raw object ID 10985 for the Peppol handler will now resolve to a different, unrelated object. Prefer picking an unused ID for the new object instead of reusing a just-vacated one, or add an explicit release note calling out the internal ID reuse.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| { | ||
| fields | ||
| { | ||
| field(10970; "FR Identifier Scheme"; Enum "Electronic Address Scheme") |
There was a problem hiding this comment.
The new Service Participant field uses enum "Electronic Address Scheme" with InitValue = ' ', but the enum's zero value is EM, not a blank sentinel. On schema sync, existing Service Participant rows will therefore deserialize the new field as EM instead of blank, because InitValue does not migrate stored rows. That can silently mark legacy participants as having an email scheme and trip the French completeness check or export the wrong identifier scheme. Use a true zero-valued blank/unknown member, or add an upgrade migration that normalizes pre-existing records explicitly.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| (FREInvoiceLifecycle."Detailed Ledger Entry No." <> DetailedLedgerEntryNo) or | ||
| (FREInvoiceLifecycle."Original Occurrence Entry No." <> OriginalOccurrenceEntryNo) | ||
| then | ||
| Error(ConflictingReplayErr); |
There was a problem hiding this comment.
VerifyReplay raises a plain client-visible Error when the same source occurrence is replayed with conflicting regulatory values. That is an internal caller-contract violation the end user cannot act on, so it should be raised as an Internal ErrorInfo instead of exposing the raw invariant message.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
RaiseInternalError(ConflictingReplayErr);Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| { | ||
| addlast(Processing) | ||
| { | ||
| action(ViewFREInvoiceLifecycles) |
There was a problem hiding this comment.
The new action(ViewFREInvoiceLifecycles) added to "E-Documents" does not use a clear reserved prefix or suffix; embedding FR inside the identifier is not the same as applying the app's affix to a member added to another app's page. AppSource guidance requires extension actions to carry the registered affix so they cannot collide with actions from other extensions.
Knowledge:
- microsoft/knowledge/appsource/object-affixes-prevent-collisions.md
- microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| @@ -1,3 +1,11 @@ | |||
| namespace Microsoft.eServices.EDocument.Formats.Test; | |||
There was a problem hiding this comment.
This existing test-app codeunit now declares namespace Microsoft.eServices.EDocument.Formats.Test, changing the published identity of codeunit 148149 "E-Doc. FR Struct. Import Tests" from an unnamespaced symbol to a namespaced one. Any dependent extension that referenced the old identity will stop compiling; keep the shipped namespace unchanged instead of renaming it in place.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
| field(5; "Currency Code"; Code[10]) | ||
| { | ||
| Caption = 'Currency Code'; | ||
| DataClassification = CustomerContent; | ||
| ToolTip = 'Specifies the currency code for the reported amount.'; | ||
| } |
There was a problem hiding this comment.
Field "Currency Code" stores the currency for the VAT breakdown amount but does not declare a TableRelation to the Currency master, so the table can hold invalid currency codes that do not exist in Business Central. Add TableRelation = Currency to keep the amount's currency reference explicit and valid.
| field(5; "Currency Code"; Code[10]) | |
| { | |
| Caption = 'Currency Code'; | |
| DataClassification = CustomerContent; | |
| ToolTip = 'Specifies the currency code for the reported amount.'; | |
| } | |
| field(5; "Currency Code"; Code[10]) | |
| { | |
| Caption = 'Currency Code'; | |
| DataClassification = CustomerContent; | |
| TableRelation = Currency; | |
| ToolTip = 'Specifies the currency code for the reported amount.'; | |
| } |
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
Why
French e-reporting regulation requires that when a payment is applied to an e-invoice, the
Collectedlifecycle status must be captured and reported to the Portail Public de Facturation (PPF). Currently, the EReporting FR app has no mechanism to track payment applications against e-invoices or to build the required XML lifecycle messages. This gap blocks compliance with the French e-invoicing mandate.Summary
FR E-Invoice Lifecycletable to capture immutable lifecycle occurrences (payment applications, reversals) linked to e-documents and customer ledger entriesFR E-Invoice Lifecycle VATtable to store per-occurrence VAT breakdown lines with amounts and currencyFR E-Invoice Lifecycle Mgt.codeunit that subscribes toOnAfterInsertDtldCustLedgEntryandOnAfterInsertDtldCustLedgEntryUnapplyto automatically capture payment lifecycle eventsFR E-Invoice Lifecycle Msg.codeunit implementingIEDocMessageBuilderto produce CrossDomainAcknowledgementAndResponse XML messages per the French regulatory profileFR E-Invoice Lifecycle ErrorandFR E-Invoice Lifecycle Workercodeunits for asynchronous processing with error handlingFR E-Invoice Lifecycleslist page for viewing captured occurrencesFixes
AB#637593