-
Notifications
You must be signed in to change notification settings - Fork 685
[E-Documents Core] - Migrate electronic formats to new purchase draft concept for e-invoicing (e-invoice preview) (PEPPOL) #28983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Introduced new enum and interface for handling Purchase Credit Memos. - Updated Purchase Invoice codeunit to validate and handle additional fields. - Enhanced E-Document Purchase Header table to include E-Document Type. - Implemented logic in PEPPOL handler for processing Credit Notes. - Added methods for populating Credit Memo headers and lines in XML. - Refactored existing methods to utilize a new XML helper codeunit for cleaner code. - Updated unit of measure provider interface to include retrieval of default unit of measure.
Changes the purchase line lookup from FindFirst to FindLast to ensure proper line number sequencing when creating new purchase lines. Moves the CreatePurchaseLine method to the end of the file for better code organization.
Improves code maintainability by adding comprehensive XML documentation comments to helper methods in the E-Document module. Documents parameter types, descriptions, and return values for XML parsing utilities and purchase credit memo creation procedures to enhance developer experience and code understanding.
Use next available ID (recent MSFT changes introduced new obects)
Changes codeunit number from 6175 to 6177 to resolve potential ID conflicts. Alphabetizes using statements across multiple files to improve code organization and maintainability. Adds missing blank lines in enum definitions for better formatting consistency. Enhances documentation by adding parameter descriptions to interface method.
Improves code formatting by adding a blank line after the exit statement to enhance visual separation between the early return and subsequent logic flow.
|
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. |
Sets default unit of measure when missing during purchase line processing to prevent validation errors. Clears item reference number when purchase type changes to maintain data consistency and avoid mismatched references.
Groenbech96
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good work on this. We have reviewed and left some feedback that should be addressed.
Apps/W1/EDocument/app/src/Processing/Import/FinishDraft/EDocCreatePurchCrMemo.Codeunit.al
Outdated
Show resolved
Hide resolved
Apps/W1/EDocument/app/src/Processing/Import/FinishDraft/EDocCreatePurchCrMemo.Codeunit.al
Outdated
Show resolved
Hide resolved
| end; | ||
|
|
||
| PurchaseHeader.Validate("Vendor Invoice No.", VendorInvoiceNo); | ||
| PurchaseHeader.Validate("Vendor Order No.", EDocumentPurchaseHeader."Purchase Order No."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vendor Order No. refers to external number for seller. Purchase Order No. on EDocumentPurchaseHeader refers to internal numbering system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I undersand the issue @Groenbech96.
codeunit 6166 "EDoc Import PEPPOL BIS 3.0" reads Invoice/cac:OrderReference/cbc:ID into Purchase Header field "Vendor Order No."
codeunit 6140 "E-Doc. Import" procedure V1_CopyFromPurchaseHeader also assigns information to E-Document Purchase Header field "Purchase Order No." from Purchase Header field "Vendor Order No."
As I understand this should be "our" reference, or I misunderstood this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here is why i think it is wrong.
https://docs.peppol.eu/poacc/billing/3.0/syntax/ubl-invoice/cac-OrderReference/cbc-ID/
States that it is issued by Buyer.
https://learn.microsoft.com/en-us/dynamics365/business-central/purchasing-how-record-purchases#external-document-number
States that "On purchase documents and journals, you can specify a document number that refers to the vendor's numbering system." Aka, issued by Seller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we might have had a bug for some time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so as agreed, we'll keep this as it is now:
Invoice/cbc:ID is used to set "Sales Invoice No." on E-Document Purchase Header which then set as "Vendor Invoice No."/"Vendor Cr. Memo No." on the Purchase Header
cac:OrderReference/cbc:ID is used to set "Purchase Order No." on E-Document Purchase Header which then set as "Vendor Order No." on the Purchase Header
...ment/app/src/Processing/Import/StructureReceivedEDocument/EDocumentPEPPOLHandler.Codeunit.al
Outdated
Show resolved
Hide resolved
...ment/app/src/Processing/Import/StructureReceivedEDocument/EDocumentPEPPOLHandler.Codeunit.al
Show resolved
Hide resolved
Apps/W1/EDocument/app/src/Processing/Import/FinishDraft/EDocCreatePurchCrMemo.Enum.al
Outdated
Show resolved
Hide resolved
Apps/W1/EDocument/app/src/Processing/Import/FinishDraft/EDocCreatePurchaseInvoice.Enum.al
Show resolved
Hide resolved
Apps/W1/EDocument/app/src/Processing/Import/ImportEDocumentProcess.Codeunit.al
Outdated
Show resolved
Hide resolved
| if PurchaseHeader.Get("Purchase Document Type"::Order, EDocumentPurchaseHeader."Purchase Order No.") then; | ||
| Vendor.SetLoadFields("Receive E-Document To"); | ||
| Vendor.Get(EDocumentPurchaseHeader."[BC] Vendor No."); | ||
| case Vendor."Receive E-Document To" of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should exit if Receive to invoice. This function should never return a purchase order. :)
I think we can keep it at that for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the original code was retrieving Order - I've just implemented the logic to retrieve the document type based on settings on Vendor card.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but this entire function is called GetPurchaseOrder.
It Maybe it should not be called at the callsite, based on that setting, but returning an invoice is confusing and not intention of the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I revert my changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code updated to be more consistent with naming
Apps/W1/EDocument/app/src/Processing/Import/PrepareDraft/EDocProviders.Codeunit.al
Outdated
Show resolved
Hide resolved
Adds three new test methods to validate PEPPOL invoice processing workflow including data extraction viewing, purchase invoice creation, and draft modification scenarios. Improves test data consistency by introducing mock currency and date variables that can be configured across test methods rather than hardcoded values. Fixes page validation trigger formatting by removing unnecessary page update call and correcting indentation. Strengthens assertion messages with detailed error information including field names, table names, expected and actual values for better debugging.
Consolidates duplicate code between invoice and credit memo creation into a shared helper class to improve maintainability and consistency. Adds ResetDraft method to structured format readers for proper cleanup when undoing processing steps. Removes obsolete enum and simplifies unit of measure handling by moving default logic to item reference processing. Enhances credit memo creation with applies-to document support for better document linking.
Introduces a utility method that simplifies extracting text content from XML nodes using XPath expressions. The method handles node selection and safely returns the inner text value or an empty string if the node is not found, reducing boilerplate code for XML parsing operations.
Consolidates duplicate draft reset code by moving the implementation to a central ResetDraft method on the E-Document table. Multiple codeunits previously contained identical logic for deleting purchase headers, lines, mappings, and line fields. Improves maintainability by eliminating code duplication and providing a single source of truth for draft cleanup operations.
Corrects the assignment of purchase order number and applies-to document number fields in credit note XML structure. Previously, the values were swapped between OrderReference and BillingReference sections, causing incorrect document references in generated PEPPOL credit notes.
Adds empty checks before calling delete operations to avoid potential runtime errors when attempting to delete records that don't exist. Improves robustness by ensuring delete operations are only performed when there are actual records to delete.
…ons.Codeunit.al Co-authored-by: Grasiele Matuleviciute <131970463+GMatuleviciute@users.noreply.github.com>
….Codeunit.al Co-authored-by: Grasiele Matuleviciute <131970463+GMatuleviciute@users.noreply.github.com>
….Codeunit.al Co-authored-by: Grasiele Matuleviciute <131970463+GMatuleviciute@users.noreply.github.com>
….Codeunit.al Co-authored-by: Grasiele Matuleviciute <131970463+GMatuleviciute@users.noreply.github.com>
….Codeunit.al Co-authored-by: Grasiele Matuleviciute <131970463+GMatuleviciute@users.noreply.github.com>
….Codeunit.al Co-authored-by: Grasiele Matuleviciute <131970463+GMatuleviciute@users.noreply.github.com>
….Codeunit.al Co-authored-by: Grasiele Matuleviciute <131970463+GMatuleviciute@users.noreply.github.com>
….Codeunit.al Co-authored-by: Grasiele Matuleviciute <131970463+GMatuleviciute@users.noreply.github.com>
| if PurchaseHeader.Get("Purchase Document Type"::Order, EDocumentPurchaseHeader."Purchase Order No.") then; | ||
| Vendor.SetLoadFields("Receive E-Document To"); | ||
| Vendor.Get(EDocumentPurchaseHeader."[BC] Vendor No."); | ||
| case Vendor."Receive E-Document To" of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but this entire function is called GetPurchaseOrder.
It Maybe it should not be called at the callsite, based on that setting, but returning an invoice is confusing and not intention of the function.
|
There is also a merge conflict. |
Refactors validation helpers to avoid shared state by passing required data explicitly, reducing test coupling and flakiness. Unifies and locks label tokens, standardizes error messages, and fixes API usage for captions. Cleans up unused variables and minor formatting. Updates tests to use the new helper signature.
Moves draft cleanup from the document level to the purchase header, deleting header, lines, header mappings, and additional line fields in one place. Updates import handlers to fetch the purchase header from the document and invoke the new reset, improving encapsulation and avoiding partial cleanup. Aligns a history-mapping call with the updated API naming for clarity.
|
merge conflict resolved |
…mework Implements comprehensive PEPPOL e-document validation improvements including: - Stateless and consistent PEPPOL validation implementation - Enhanced structured e-document testing framework - Improved test coverage for PEPPOL validation scenarios - Refined validation logic for better consistency across operations
Moves draft cleanup to the record delete trigger to always remove related lines and mappings when discarding a draft, removing the custom reset method and updating callers to delete the header directly. Splits purchase document lookup by exposing separate methods for order and invoice. The caller now chooses based on vendor setup, decoupling provider logic from vendor configuration and avoiding provider-level validation errors. Minor refactor in draft preparation to handle the new lookup and streamline flow.
| end; | ||
|
|
||
| PurchaseOrder := IPurchaseOrderProvider.GetPurchaseOrder(EDocumentPurchaseHeader); | ||
| case Vendor."Receive E-Document To" of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use this in the exit clause on line 62.
exit(if Vendor.Receive E-Document To = Purchase Order ? E-Document Type::PO : E-Document Type::Invoice)
Then the rest can be left as it was. This setting should probably only influence that bit. Even if we have invoice, it would be okay to fill [BC] Purchase Order No.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Updates draft preparation to honor the vendor’s target (order or invoice), validating document type and returning the corresponding e-document type instead of always using order. Ensures purchase header data is initialized from the e-document before processing to prevent missing header values. Adapts tests to the parameter-based import call and updates helpers/mocks accordingly. Adds attachment dependency for purchase invoice creation.
AndriusAndrulevicius
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All comments should be now resolved.
Automated tests also included in this PR
|
Processing this PR. The branch is now locked 🔒 Please don't push updates unless otherwise agreed. |
|
Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one. Build ID: . |
This pull request does not have a related issue as it's part of the delivery for development agreed directly with @altotovi @Groenbech96
Implementation
Fixes #
Fixes AB#598453