[Main]Customer Direct Debit Payment Slip – SEPA File Generation Fails and Credit Memo Is Processed Incorrectly When Invoice and Credit Memo Exist for the Same Customer in the French version. - #10895
Conversation
…irect Debit collection
…ing and enhance SEPA file export functionality
…streamline invoice retrieval logic
…Bug-641369-main-Cust-Pmt-Slip-SEPA-File-Fails-Cr-Memo-Process-Incorrect
|
The new posting test says it proves both the bank debit and the receivables credit, but VerifyGenLedgerEntry only filters rows where "Debit Amount" is non-zero and sums those debits. A regression that posts the right debit total while misposting or omitting the credit-side receivables entry would still pass. Add an assertion for the corresponding credit entry and, ideally, the expected receivables account. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4 |
Good Sense Reviewer - Round 1Recommendation: Request ChangesWhat this PR doesThis change tries to let French customer payment slips net a credit memo against an invoice for the same customer, both when posting the slip and when creating the SEPA direct debit file. The main path is covered: the export now finds one invoice entry and subtracts a credit memo amount, and the posting path moves the credit memo application onto the shared invoice application. The remaining problem is that the new netting checks are wider than the reported scenario. They can skip or combine credit memo lines that are not proven to belong to the same invoice netting group. Problem-solution fitFit: Partial The reported scenario is important and the change addresses the simple invoice-plus-credit-memo case. It does not yet preserve the existing validation for standalone or unrelated credit memo lines, so the solution is not safe enough for this payment-file and posting path. SuggestionsS1 (🔴 High): Credit memo lines can be skipped silently S2 (🔴 High): Netting scans unrelated customer lines S3 (🟠 Moderate): Posting test misses the credit entry Risk assessment and necessityRisk: This is a high-risk payment and posting change. It affects SEPA direct debit export, customer ledger application, and G/L signs in Necessity: The change is needed because collecting the gross invoice while leaving the credit memo open is wrong. The scope is also in the right area, but it must keep the old error behavior unless a credit memo is safely netted into the same invoice collection.
|
…Bug-641369-main-Cust-Pmt-Slip-SEPA-File-Fails-Cr-Memo-Process-Incorrect
Good Sense Reviewer - Round 2Recommendation: AcceptWhat this PR doesThe new commit tightens the customer netting check so credit memos are only skipped when they share the same Applies-to ID with an invoice. It also adds a receivables credit assertion to the posting test. With these changes, the reported invoice-plus-credit-memo collection path is narrower, and the previous sign-test gap is covered. Status of previous suggestions
New observations (commits since round 1)None - changes only addressed prior suggestions. Risk assessment and necessityRisk: This remains a sensitive payment and posting area because it affects SEPA direct debit export, customer ledger application, and G/L signs. The new commit reduces the main regression risk by keeping unrelated credit memo lines on the old error path and by checking the receivables credit entry in the posting test. Necessity: The change is needed because collecting the gross invoice while leaving the credit memo open is wrong. The scope now matches the reported invoice-and-credit-memo netting scenario more closely.
|
…Bug-641369-main-Cust-Pmt-Slip-SEPA-File-Fails-Cr-Memo-Process-Incorrect
| CreditMemoAmount: Decimal; | ||
| begin | ||
| if InvPostingBuffer[1]."Account Type" <> InvPostingBuffer[1]."Account Type"::Customer then | ||
| exit; |
There was a problem hiding this comment.
NetCustomerSettlement, TryGetCustomerNettingContext, GetSettlementAmount and ApplyCreditMemoToSharedInvoice implement a non-obvious cross-document netting algorithm (matching a credit memo against a shared invoice line, flipping sign/document type to Refund when no invoice is found, and re-pointing the ledger entry's Applies-to ID) with no explanatory comment. Because the logic silently changes posted amounts and document types based on implicit ordering assumptions (first Invoice line found via FindSet order), a short comment describing the netting contract and its ordering assumption would materially reduce the risk of future regressions during maintenance.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.40.6
| begin | ||
| DocumentCustLedgEntry.SetRange("Customer No.", PaymentLine."Account No."); | ||
| DocumentCustLedgEntry.SetRange("Document Type", DocumentType); | ||
| DocumentCustLedgEntry.SetRange("Document No.", DocumentNo); |
There was a problem hiding this comment.
ApplyCreditMemoToSharedInvoice (PaymentManagementFR.Codeunit.al) writes "Applies-to ID" directly onto a Cust. Ledger Entry and calls Modify(), bypassing the standard customer-application routines (e.g. CustEntrySetApplID / apply-and-post helpers) that also maintain detail ledger entries, currency application, and open-status bookkeeping consistently. Doing this ad hoc inside invoice-posting-buffer generation risks leaving the ledger entry's applies-to state inconsistent with the rest of the application machinery if another line targets the same invoice concurrently in the same posting run, since the field is overwritten rather than merged/validated through the normal application API. Recommend routing this through the standard apply-customer-entries logic (or at least validating no conflicting Applies-to ID is being clobbered) instead of a raw field write.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.40.6
| end; | ||
|
|
||
| #if not CLEAN28 | ||
| [EventSubscriber(ObjectType::Codeunit, Codeunit::"Payment Management Feature FR", OnAfterCheckFeatureEnabled, '', false, false)] |
There was a problem hiding this comment.
The new subscriber in "Library - Localization FR" is a default static subscriber that forces "Payment Management Feature FR" on for every event raise in every test session. That behavior is scoped test setup, so keeping it static leaks the override across unrelated tests with no way to switch it off; move it behind a manual subscriber and bind it only for the tests that need the feature enabled.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.40.6
| end; | ||
| end; | ||
|
|
||
| internal procedure TryGetCustomerNettingContext(var SharedInvoiceLine: Record "Payment Line FR"; var CreditMemoAmount: Decimal): Boolean |
There was a problem hiding this comment.
TryGetCustomerNettingContext opens a new Payment Line FR FindSet every time the caller needs netting data. The new posting and SEPA-export paths call this helper from per-line loops, so summarized slips rescan the same customer lines repeatedly instead of computing the netting context once per header/customer and reusing it.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.40.6
| end; | ||
|
|
||
| [Test] | ||
| [HandlerFunctions('PaymentClassListModalPageHandler,SuggestCustomerPaymentsSummarizedRequestPageHandler,ConfirmHandlerTrue')] |
There was a problem hiding this comment.
The new test lists PaymentClassListModalPageHandler in [HandlerFunctions(...)], but its changed path creates a header, opens an existing payment slip, runs Suggest Customer Payments, and posts the slip directly; it never opens Payment Class List. Nonoptional listed handlers must execute, so this can fail on handler wiring instead of verifying the payment-netting behavior.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
[HandlerFunctions('SuggestCustomerPaymentsSummarizedRequestPageHandler,ConfirmHandlerTrue')]Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.40.6
| PaymentClass.Get(SetupForPaymentSlipPost(PaymentStepLedger."Detail Level"::Account, PaymentClass.Suggestions::Customer)); | ||
| CreatePaymentHeader(PaymentHeader); | ||
| Commit(); // Required for execute report. | ||
| SuggestCustomerPaymentLinesSummarized(CustomerNo, SummarizePer::" ", PaymentHeader."No."); |
There was a problem hiding this comment.
The scenario in PostPaymentSlipNetsCreditMemoAgainstInvoice says it verifies "Summarize per Customer" and a single net payment line, but the test actually enqueues SummarizePer::" " and then asserts two payment lines. That means it is not covering the summarized path it claims; either drive SummarizePer::Customer and assert one line, or rename and reword the test to match the non-summarized flow.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.40.6
Good Sense Reviewer - Round 3Recommendation: Request ChangesWhat this PR doesThe new commit changes the netting lookup so it can handle separate invoice and credit-memo payment lines for the same customer. That fixes the simple non-summarized case, but it removes the grouping guard that made the previous round safe. The current code again treats any credit memo for the customer on the slip as part of the first invoice netting context, so the fix is too broad for this payment-file and posting path. Status of previous suggestions
New observations (commits since round 2)S1 (🔴 High): Credit memo lines can be skipped silently S2 (🔴 High): Netting scans unrelated customer lines Risk assessment and necessityRisk: This is still a high-risk payment and posting change. It affects SEPA direct debit amounts, customer ledger application, and receivables G/L signs in PaymentManagementFR.Codeunit.al, PaymentMgtSubscribersFR.Codeunit.al, and PaymentLineFR.Table.al. The newest change widens the netting set again, so a customer with more than the simple one-invoice/one-credit-memo case can be collected or applied incorrectly. Necessity: The fix is needed because collecting the gross invoice and leaving the credit memo open is wrong. The scope is right, but the implementation must keep unrelated documents on the old validation path unless they are safely tied to the same netting group.
|
Fixes: AB#644993
Issue
When a customer had an open invoice and an unapplied credit memo, the Customer Payment Slip (FR) handled them incorrectly:
SEPA Direct Debit export failed with errors such as "You cannot export a SEPA customer payment that is applied to multiple documents" and "unapplied lines not allowed" whenever a credit memo was collected alongside an invoice for the same customer.
Posting produced the wrong result — the credit memo was posted as a separate Refund entry instead of being netted against the invoice, so the customer was debited the gross invoice amount rather than the net balance, and entries were left open.
Root cause
Posting side (Payment Management FR): settlement amounts were always taken as Abs(...) regardless of the applied document type, so a credit memo was never subtracted. There was no logic to net a credit memo into the invoice it shared collection with, so each document posted on its own line and the credit memo became a refund.
SEPA export side (PaymentMgt Subscribers FR): CheckPaymentLine required every collected line to resolve to a single invoice ledger entry. Credit-memo lines, and invoice+credit-memo combinations under one Applies-to ID, tripped the "multiple documents" / "unapplied lines" validations. The transfer amount also used the raw Credit Amount without deducting the credit memo, so the exported amount would have been wrong even if validation passed.
Solutions
Added TryGetCustomerNettingContext on the Payment Line FR table as the single source of truth that locates the shared invoice line and aggregates the customer's credit-memo amount — reused by both the posting and export paths.
Posting (Payment Management FR):
GetSettlementAmount now signs the amount by document type (credit memo → negative).
NormalizeAccountLevelSign corrects the account-level debit/credit sign.
NetCustomerSettlement redirects the entry onto the shared invoice's Applies-to ID so invoice and credit memo settle as one net payment; a standalone credit memo (no shared invoice) falls back to a Refund.
ApplyCreditMemoToSharedInvoice applies the collected credit-memo/secondary document to the shared invoice application on the customer ledger.
SEPA export (PaymentMgt Subscribers FR):
Credit-memo lines are excluded from the file and their amount is netted into the shared invoice line via GetNetTransferAmount.
CheckPaymentLine now filters to invoice ledger entries and no longer errors on the invoice + credit-memo combination.
Tests: added coverage for the net posting flow (ERM Payment Management) and for SEPA export of an invoice + credit memo for the same customer (SEPA.02 DD Functional Test).