PEPPOL FR improvements - #11096
Conversation
GitHub Actions
Inactive
|
Could not find linked issues in the pull request description. Please make sure the pull request description contains a line that contains 'Fixes #' followed by the issue number being fixed. Use that pattern for every issue you want to link. |
github-actions
Bot
added
Team: Finance
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Good Sense Reviewer - Round 1Recommendation: Accept with SuggestionsWhat this PR doesThis PR updates the French PEPPOL export so the generated UBL uses the French customization identifiers, billing profiles, buyer and supplier identifiers, endpoint schemes, credit memo billing references, Extended CTC line references, delivery data, and grouped regulatory notes expected for French e-invoicing. The approach is targeted to the French post-processing codeunit. The main branches look coherent: service participant routing wins before customer fallback, buyer legal identifiers are derived from SIREN/SIRET/French VAT data, and Extended CTC is selected only when the invoice has multiple order, shipment, or delivery-date evidence. The added XML tests cover the main changed paths. Problem-solution fitFit: Strong The bug describes missing or incorrect French e-invoicing identifiers, references, routing, and regulatory note handling. The diff addresses those cases directly in the FR PEPPOL export and adds focused tests for the expected XML output. SuggestionsS1 (🟠 Moderate): Keep spaces between joined comment lines Risk assessment and necessityRisk: The regression surface is limited to the French PEPPOL export XML. The risky parts are XML element order, changed customization IDs, endpoint scheme preservation, and using current customer/service-participant data for routing identifiers. The change does not post amounts, update ledger entries, change schema, or remove existing integration events; it adds one new integration event. Necessity: The change is needed for French e-invoicing interoperability. The scope is appropriate because it keeps the behavior in the FR format layer and backs the main scenarios with XML assertions.
|
|
Addressed Good Sense suggestion S1 in ba98958: regulatory comment rows are trimmed and joined with exactly one space. The regression test now stores adjacent fragments without trailing or leading whitespace and verifies the exported PMD note preserves the word boundary. |
This reverts commit 9d75d30.
Good Sense Reviewer - Round 2Recommendation: Request ChangesWhat this PR doesThe new commits keep the French PEPPOL export updates and add the shipment external-document fallback for Extended CTC line references. They also fix the previous regulatory-comment spacing issue by joining non-empty trimmed fragments with one space. The spacing fix is addressed, but the new cached buyer-reference path changes one edge case. A shipment can be part of an Extended CTC invoice without having a buyer reference, and the current lookup treats that missing optional value as an export error. Problem-solution fitFit: Partial The requested French PEPPOL behavior is mostly implemented, and the previous spacing issue is fixed. The buyer-reference cache does not preserve the valid blank-reference case, so the solution is not complete yet. Status of previous suggestions
New observations (commits since round 1)S2 (🔴 High): Handle missing shipment buyer references Risk assessment and necessityRisk: The regression surface is limited to the French PEPPOL XML export. The new missing-key path can fail export for Extended CTC invoices that have shipments but no buyer reference on those shipments. Necessity: The change is needed for French e-invoicing interoperability. The scope is still appropriate, but the buyer-reference cache must keep the previous blank-reference behavior instead of failing the export.
|
|
The new test hand-builds two "Sales Comment Line" fixtures with direct field assignment, hard-coded line numbers, and Insert calls even though "Library - Sales" already provides CreateSalesCommentLine and is used elsewhere in the same file. That bypasses the maintained test-library setup for valid comment lines and makes the scenario brittle to numbering or schema changes; create both comment lines through the library and then modify only the fields under test. Suggested fix (apply manually — could not be anchored as a one-click suggestion): LibrarySales.CreateSalesCommentLine(SalesCommentLine, "Sales Document Type"::Invoice, InvoiceNo, 0);
SalesCommentLine.Validate(Comment, OrdinaryCommentText);
SalesCommentLine.Modify(true);
LibrarySales.CreateSalesCommentLine(SalesCommentLine, "Sales Document Type"::Invoice, InvoiceNo, 0);
SalesCommentLine.Validate("FR Regulatory Comment Type", SalesCommentLine."FR Regulatory Comment Type"::AAB);
SalesCommentLine.Validate(Comment, CommentText);
SalesCommentLine.Modify(true);Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.38.6 |
Good Sense Reviewer - Round 3Recommendation: Request ChangesWhat this PR doesThe latest changes keep the French PEPPOL export updates, make the customer electronic address scheme visible again, and change regulatory comment joining to trim fragments and append them directly. The buyer-reference issue from the previous round is still present, and the regulatory-comment separator issue is reintroduced. Problem-solution fitFit: Partial The requested French PEPPOL behavior is mostly implemented, but the Extended CTC shipment buyer-reference path can still fail for a valid blank-reference case. The regulatory-comment aggregation also still risks producing invalid text when comment lines need a word or sentence separator. Status of previous suggestions
New observations (commits since round 2)None - the new commit only changes the state of previous findings. Risk assessment and necessityRisk: The regression surface is limited to French PEPPOL XML export. A valid Extended CTC invoice can still fail when a shipment has no buyer reference, and grouped regulatory notes can still lose required spacing. Necessity: The change is needed for French e-invoicing interoperability. The scope is appropriate, but the buyer-reference lookup must keep blank references safe before this can merge.
|
Good Sense Reviewer - Round 4Recommendation: Accept with SuggestionsWhat this PR doesThe latest change keeps the French PEPPOL export updates and fixes the Extended CTC shipment buyer-reference path. A shipment without a stored buyer reference now leaves the line reference blank instead of failing the export, while a shipment that has a reference can still add OrderLineReference. The missing-key issue from the previous round is addressed by using the Boolean return from ShipmentBuyerReferences.Get(...). The remaining regulatory-comment issue is unchanged: comment fragments of the same type are still appended without any separator. Problem-solution fitFit: Partial The main French PEPPOL requirements are implemented, and the Extended CTC blank-reference case is now handled. The regulatory-comment aggregation still does not fully preserve readable text when one tagged comment is split across multiple rows. Status of previous suggestions
New observations (commits since round 3)None - the new commit only fixes the missing shipment buyer-reference path. Risk assessment and necessityRisk: The regression surface is limited to French PEPPOL XML export. The fixed shipment lookup no longer blocks valid Extended CTC invoices with a blank shipment reference. The open risk is that grouped regulatory notes can still merge words or sentences when the text is split across comment lines. Necessity: The change is needed for French e-invoicing interoperability. The scope remains appropriate because it stays in the French format layer and the latest fix is narrow.
|
Good Sense Reviewer - Round 5Recommendation: Request ChangesWhat this PR doesThe latest commits keep the French PEPPOL export changes, refine the shipment buyer-reference regression test so it checks the correct invoice line, and add data-classification registration for e-document messages. The shipment buyer-reference test now matches the fixed behavior from the previous round. The regulatory-comment issue is still open, and the latest test edit introduces a new analyzer warning that makes the FR build fail. Problem-solution fitFit: Partial The main French PEPPOL requirements are mostly implemented, and the blank shipment-reference case is now covered more precisely. The solution is not merge-ready because one existing text-joining issue remains and the current branch does not pass the required FR build checks. Status of previous suggestions
New observations (commits since round 4)S3 (🔴 High): Fix the new AA0217 build warning Risk assessment and necessityRisk: The latest production change is limited to data-classification setup, and the test-only XPath change does not affect runtime XML output. The current merge risk is practical: the FR build is failing, and grouped regulatory notes can still merge words or sentences when one legal note is split across comment lines. No BaseApp publisher dependency or Sales/Purchase/Service parallel change was introduced in this round. Necessity: The French PEPPOL updates are needed for e-invoicing interoperability, including identifiers, routing, billing references, delivery data, and Extended CTC France output. The scope is still appropriate for the French format layer, but the build warning must be fixed before merge.
|
Good Sense Reviewer - Round 6Recommendation: Request ChangesWhat this PR doesThe latest commit only moves the invoice-line XPath format string from inline text to a locked label in the PEPPOL FR tests. That matches the previous build-warning fix and does not change runtime XML output. The branch now passes the FR build checks that were failing before. One previous runtime issue remains: split regulatory comments of the same type are still trimmed and appended without any separator, so two parts of one legal note can be merged into one word or sentence. Problem-solution fitFit: Partial The French PEPPOL requirements are mostly covered, and the latest test-only change fixes the build-warning problem. The solution is still not merge-ready because the regulatory-comment joining behavior can change the legal text that is exported. Status of previous suggestions
New observations (commits since round 5)None - the only new changed span is the test label used by StrSubstNo, and it matches the required build-warning fix. Risk assessment and necessityRisk: The latest commit is test-only, so it does not add runtime regression risk. The remaining risk is in the FR PEPPOL export: grouped regulatory notes can still merge words or sentences when one required legal note is split over multiple sales comment lines. No BaseApp publisher dependency or Sales/Purchase/Service parallel change was introduced in this round. Necessity: The French PEPPOL updates are needed for e-invoicing interoperability, including identifiers, routing, billing references, delivery data, and Extended CTC France output. The latest build fix was necessary, but the exported regulatory note text still needs a separator-safe join before merge.
|
Pull request was closed
Pull request was closed
2 participants
Why
French PEPPOL exports need to meet partner feedback and French e-invoicing requirements for identifiers, billing profiles, regulatory comments, document references, and participant routing. The generated UBL must also preserve source order and delivery information when Extended CTC France is required.
Summary
Fixes
AB#647916