Skip to content

[E-Document Formats] Migrate NAV PRs 247170 and 247176 into BCApps#9426

Open
Groenbech96 wants to merge 3 commits into
mainfrom
user/magnushar/migrate-nav-pr-247170-247176
Open

[E-Document Formats] Migrate NAV PRs 247170 and 247176 into BCApps#9426
Groenbech96 wants to merge 3 commits into
mainfrom
user/magnushar/migrate-nav-pr-247170-247176

Conversation

@Groenbech96

@Groenbech96 Groenbech96 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Groenbech96 Groenbech96 requested review from a team July 14, 2026 11:53
@Groenbech96 Groenbech96 requested review from a team as code owners July 14, 2026 11:53
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Jul 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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'

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 2 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@809af9708e265f110ced6752fd8f2e40f1efc820

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain Findings Knowledge-backed Agent Inline Fallback
Agent 1 0 1 1 0
Style 2 2 0 2 0

Totals: 2 knowledge-backed · 1 agent findings.

Orchestrator pre-filter (2 file(s) excluded)

  • layer-disabled (knowledge) : 2 file(s)

Findings produced by the Copilot CLI agent against BCQuality at 809af9708e265f110ced6752fd8f2e40f1efc820. Reply 👎 on any inline comment to flag false positives.

Magnus Hartvig Grønbech and others added 2 commits July 14, 2026 17:58
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>
@Groenbech96

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

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

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

Labels

AL: Apps (W1) Add-on apps for W1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant