Skip to content

Add GTIN to XRechnung and ZUGFeRD exports - #10048

Open
Milica Đukić (djukicmilica) wants to merge 6 commits into
mainfrom
bugs/GTIN-XRechnung-ZUGFeRD
Open

Add GTIN to XRechnung and ZUGFeRD exports#10048
Milica Đukić (djukicmilica) wants to merge 6 commits into
mainfrom
bugs/GTIN-XRechnung-ZUGFeRD

Conversation

@djukicmilica

@djukicmilica Milica Đukić (djukicmilica) commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Why

XRechnung and ZUGFeRD exports omit the GTIN stored on item cards, preventing recipients in retail and service scenarios from identifying invoiced items by their standard product identifier.

Summary

  • Added GTIN output with GS1 scheme 0160 to XRechnung and ZUGFeRD item lines.
  • Applied the same behavior to sales invoices and credit memos while omitting blank GTIN values.
  • Added focused XML regression coverage for both formats.

Fixes

AB#646116

@djukicmilica
Milica Đukić (djukicmilica) requested a review from a team August 7, 2026 12:37
@djukicmilica
Milica Đukić (djukicmilica) requested a review from a team as a code owner August 7, 2026 12:37
@djukicmilica Milica Đukić (djukicmilica) added the Finance GitHub request for Finance area label Aug 7, 2026
@github-actions github-actions Bot added Integration GitHub request for Integration area and removed Finance GitHub request for Finance area labels Aug 7, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 7, 2026
Comment thread src/Apps/DE/EDocumentDE/app/src/XRechnung/ExportXRechnungDocument.Codeunit.al Outdated
Comment thread src/Apps/DE/EDocumentDE/app/src/ZUGFeRD/ExportZUGFeRDDocument.Codeunit.al Outdated
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

GetAttributeByPathWithError introduces a hardcoded Error('Attribute %1 not found for node: %2', ...) call. Per the referenced guidance, error text should be declared as a Label and passed to Error(...) with its parameters directly; keeping the format string inline makes the diagnostic untranslatable and opaque to AL analyzers. Replace the literal with an ...Err label and call Error(TheLabelErr, AttributeName, ElementXPath).

Knowledge:

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

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

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

The new ItemGTINCache field in ExportXRechnungDocument.Codeunit.al is only reset via Clear(ItemGTINCache) at the start of the two sales-document CreateXML overloads (Sales Invoice Header, Sales Cr.Memo Header). The Service Invoice Header and Service Cr.Memo Header CreateXML overloads (which also emit item lines through InsertInvoiceLines/InsertStandardItemIdentification via temporary sales lines transferred from service lines) do not clear the cache. If the same codeunit instance is reused across multiple CreateXML calls within a batch/session (e.g. export sales invoice then service invoice, or repeated calls), a stale cached GTIN keyed by Item No. could be returned instead of re-reading the current Item.GTIN value. Clear the cache at the start of every CreateXML overload that can emit item lines, not just the two sales ones.

Agent judgement — not directly backed by a BCQuality knowledge article.

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

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

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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

This PR adds a new InsertGlobalID(SpecifiedTradeProductElement, ItemNo) call in InsertInvoiceLine/InsertCrMemoLine (ExportZUGFeRDDocument.Codeunit.al) that writes Item.GTIN into /rsm:CrossIndustryInvoice/.../ram:SpecifiedTradeProduct/ram:GlobalID whenever the line is Type::Item and the referenced Item has a GTIN. However, four pre-existing tests in ZUGFeRDXMLDocumentTests.Codeunit.al already assert different semantics for that exact XML path: ExportPostedSalesInvoiceInZUGFeRDFormatVerifyGlobalID (line ~641) and ExportPostedSalesCrMemoInZUGFeRDFormatVerifyGlobalID (line ~1080) assert GlobalID equals SalesInvoiceLine/SalesCrMemoLine.'Item Reference No.', which CreateAndPostSalesDocumentWithItemRefNo sets to a random GUID (LibraryUtility.GenerateGUID()) - never the Item's GTIN. After this change, the exporter no longer ever writes 'Item Reference No.' into GlobalID; it writes Item.GTIN only, and only when GTIN is populated. Since the GUID cannot equal a populated GTIN and the test items in these two pre-existing tests are not shown to have GTIN set, GetNodeByPathWithError will most likely throw 'Node not found' (or return an unrelated GTIN value), causing these two pre-existing tests to fail. This is a concrete, demonstrable regression to existing shipped test coverage introduced by this diff, not merely a hypothetical risk. Fix by reconciling the two features: either give the new GTIN-based GlobalID a different scheme/element than the one the pre-existing 'Item Reference No.' tests target, or update/remove the conflicting pre-existing tests so the suite has one consistent definition of what SpecifiedTradeProduct/GlobalID represents.

Agent judgement — not directly backed by a BCQuality knowledge article.

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

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

auto-merge was automatically disabled August 8, 2026 07:06

Pull request was closed

Comment thread src/Apps/DE/EDocumentDE/test/src/XRechnungXMLDocumentTests.Codeunit.al Outdated
Comment thread src/Apps/DE/EDocumentDE/test/src/ZUGFeRDXMLDocumentTests.Codeunit.al Outdated
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

This PR silently changes the meaning of the already-shipped ZUGFeRD line-level ram:GlobalID from Sales Line "Item Reference No." to Item.GTIN (and adds the equivalent cac:StandardItemIdentification to XRechnung for the first time). Existing customers/integrations that populated "Item Reference No." and consumed that value from the exported XML will silently receive different data after upgrade, with no compatibility switch, feature flag, or deprecation note. Consider preserving prior behavior by default (or falling back to Item Reference No. when GTIN is blank) and introduce the GTIN mapping as an explicit, documented change, or confirm with product management that this is an intentional, accepted behavior change for this feature area.

Agent judgement — not directly backed by a BCQuality knowledge article.

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

The new XRechnung export writes Item.GTIN into cac:StandardItemIdentification/cbc:ID with schemeID 0160 (GS1/GTIN), but verify this app's corresponding XRechnung/Peppol import path does not map that same element back to the internal item number or a different reference field. If an import mapping elsewhere in this app (or a downstream consumer) expects StandardItemIdentification to carry an internal item identifier rather than a GS1 GTIN, round-tripping a document exported by this codeunit could misinterpret the GTIN as an internal item reference. Confirm the export/import contract is consistent for this element across the app.

Agent judgement — not directly backed by a BCQuality knowledge article.

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

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

GetItemGTIN() is called once per posted sales line and still performs an Item.Get for every distinct item number on the document. The per-run Dictionary only avoids repeated lookups for duplicate items on the same document; documents with many different items still incur one Item.Get per distinct item (N+1 pattern). Preload GTINs for all distinct item numbers on the document in a single batched read (e.g., SetFilter on Item."No." with the distinct item numbers, or a temp-table join) before iterating lines, instead of a per-line/per-item Get.

Knowledge:

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

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

Same N+1 Item.Get-per-distinct-item pattern as the XRechnung exporter: GetItemGTIN() caches per item number but still performs one Item.Get per distinct item on the document rather than a single batched read. Preload GTINs for all distinct item numbers on the document before iterating lines.

Knowledge:

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

GetItemGTIN() (lookup + Dictionary cache logic) is duplicated nearly verbatim between Export XRechnung Document and Export ZUGFeRD Document, differing only in the XML element names used to emit the identifier. This makes it easy to fix or extend GTIN handling in one exporter (e.g., adding a fallback, changing the cache-clear scope, or handling additional identifier schemes) while missing the other. Consider extracting the shared GTIN lookup/caching logic into a common helper codeunit, keeping only the format-specific XML element construction in each exporter.

Agent judgement — not directly backed by a BCQuality knowledge article.

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Testing}$

The three SetItemGTIN helper overloads and the new GTIN test scenarios are duplicated verbatim between XRechnungXMLDocumentTests and ZUGFeRDXMLDocumentTests. Any future change to how GTIN is set up or asserted must be applied identically in both suites, or they will drift (note ZUGFeRD's new tests do not include an equivalent to XRechnung's 'OmitsBlankGTIN' test, which is already an early sign of drift). Consider a shared test helper/library for GTIN setup shared by both codeunits.

Agent judgement — not directly backed by a BCQuality knowledge article.

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

ItemGTINCache is cleared at the start of the Sales Invoice and Sales Cr.Memo CreateXML overloads but not at the start of the Service Invoice and Service Cr.Memo CreateXML overloads (both of which internally build a temporary Sales Invoice/Cr.Memo line and call the same InsertInvoiceLines/InsertStandardItemIdentification path that reads the cache). Since a single codeunit instance can be reused across multiple CreateXML calls, add Clear(ItemGTINCache); to both service overloads for consistency, even though GTIN is item-scoped so this is unlikely to produce incorrect data in practice.

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

ItemGTINCache is cleared at the start of the Sales Invoice and Sales Cr.Memo CreateXML overloads but not at the start of the Service Invoice and Service Cr.Memo CreateXML overloads, which reuse the same line-insertion path that reads the cache. Add Clear(ItemGTINCache); to both service overloads for consistency with the sales-document overloads.

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Integration GitHub request for Integration area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant