[E-Document Formats] Migrate NAV PRs 247170 and 247176 into BCApps#9426
[E-Document Formats] Migrate NAV PRs 247170 and 247176 into BCApps#9426Groenbech96 wants to merge 3 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234' |
Copilot PR ReviewIteration 2 · Outcome: completed Knowledge source: https://github.com/microsoft/BCQuality@809af9708e265f110ced6752fd8f2e40f1efc820 Findings by domainFindings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).
Totals: 2 knowledge-backed · 1 agent findings. Orchestrator pre-filter (2 file(s) excluded)
Findings produced by the Copilot CLI agent against BCQuality at |
…e-nav-pr-247170-247176
Resolves Pull Request Build failures AL0185 (missing 'EDocument XML Helper') and AL0297 (Factura-E object id range). [bcapps-fix-loop attempt 1] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Follow-up on the work-item check: I could not infer a valid ADO work item from the mirrored source PRs, so this needs a human-provided work item ID. Once an ID is provided, add AB# (or Fixes AB#) to the PR description and the check should pass. |
| EDocReadablePurchaseDoc.Run(); | ||
| end; | ||
|
|
||
| #pragma warning restore AA0139 |
There was a problem hiding this comment.
In "E-Document PINT A-NZ Handler" (codeunit 28008), the '#pragma warning restore AA0139' at line 83 appears before any matching '#pragma warning disable AA0139' in the file, and the corresponding disable only starts at line 125.
As a result, PopulateEDocumentForInvoice (lines 84-103) and PopulateEDocumentForCreditNote (lines 105-124) are left unprotected even though they call the same SetStringValueInField/SetNumberValueInField pattern (e.g. lines 90-96, 111-117) that the pragma is meant to silence, so AA0139 will fire there. Compare with the sibling EDocumentFacturaEHandler.Codeunit.al in this same PR, where every method using this pattern is correctly wrapped in its own disable/restore pair.
Recommendation:
- move '#pragma warning disable AA0139' to just before 'local procedure PopulateEDocumentForInvoice' (before line 84) and keep the existing restore at line 83's position after the block that needs it, mirroring the FacturaE handler's pairing.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
| PINTANZXml: XmlDocument; | ||
| XmlNamespaces: XmlNamespaceManager; | ||
| XmlElement: XmlElement; | ||
| CommonAggregateComponentsTok: Label 'urn:oasis:names:specification:ubl:schema:xsd:CommonAggregateComponents-2', Locked = true; |
There was a problem hiding this comment.
In "E-Document PINT A-NZ Handler" (codeunit 28008), several Label variables are declared inside procedure-local var blocks instead of the codeunit's top-level var block: CommonAggregateComponentsTok/CommonBasicComponentsTok/DefaultInvoiceTok/DefaultCreditNoteTok in ReadIntoDraft (lines 39-42), ABNSchemeIdTok in ParseAccountingSupplierPartyForPurchaseHeader (line 135), and InvoiceLinePathTok/CreditNoteLinePathTok in InsertPINTANZPurchaseInvoiceLines (lines 197-198).
The file already demonstrates the correct pattern with GLNSchemeIdTok declared at codeunit scope (line 24). Move the procedure-local Labels to the codeunit's top-level var block alongside GLNSchemeIdTok.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
| FacturaEXML: XmlDocument; | ||
| XmlNamespaces: XmlNamespaceManager; | ||
| XmlElement: XmlElement; | ||
| FacturaENamespaceTok: Label 'http://www.facturae.gob.es/formato/Versiones/Facturaev3_2_2.xml', Locked = true; |
There was a problem hiding this comment.
In "E-Document Factura-E Handler" (codeunit 10776), several Label variables are declared inside procedure-local var blocks instead of the codeunit's top-level var block: FacturaENamespaceTok/DigitalSignatureNamespaceTok/ETSINamespaceTok in ReadIntoDraft (lines 40-42), InvoiceLinePathTok in InsertFacturaEPurchaseInvoiceLines (line 110), IndividualTok in GetNameDependingOnType (line 230), and ResidenceTok in GetAddressDependingOnType (line 251).
Move these Labels to the codeunit's top-level var block so they participate reliably in the translation/XLIFF extraction pipeline.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
Source PRs\n- https://dynamicssmb2.visualstudio.com/Dynamics%20SMB/_git/NAV/pullrequest/247176\n- https://dynamicssmb2.visualstudio.com/Dynamics%20SMB/_git/NAV/pullrequest/247170\n\n## Summary\nThis combines both NAV PR migrations into a single BCApps PR from main:\n\n1. Factura-E read-into-draft handler + tests/resources.\n2. PINT A-NZ read-into-draft handler + tests/resources.\n3. Required internalsVisibleTo entries in src/Apps/W1/EDocument/App/app.json for both format apps and their test apps.\n\n## Notes\n- Kept BCApps versioning conventions in app manifests while applying the functional migration changes.\n