Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,20 @@ report 2000019 "Suggest Vendor Payments EB"
ApplicationArea = Basic, Suite;
Caption = 'Take Payment Discounts';
ToolTip = 'Specifies if you want the batch job to include vendor ledger entries for which you can receive a payment discount.';

trigger OnValidate()
begin
if (IncPmtDiscount) and (PmtDiscDueDate = 0D) then

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 the IncPmtDiscount field's OnValidate trigger, the new logic 'if (IncPmtDiscount) and (PmtDiscDueDate = 0D) then PmtDiscDueDate := WorkDate() else PmtDiscDueDate := 0D' resets PmtDiscDueDate to 0D whenever IncPmtDiscount is true but PmtDiscDueDate already holds a non-zero value (the else branch fires because the AND condition requires PmtDiscDueDate = 0D).

The prior code only ever initialized an empty date and never clobbered an already-set one. Recommend nesting the checks so a non-zero date is preserved when the checkbox is enabled, and only cleared when it is disabled: guard the WorkDate() default inside 'if IncPmtDiscount then' and only assign 0D in the 'else' for IncPmtDiscount = false.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

                            if IncPmtDiscount then begin
                                if PmtDiscDueDate = 0D then
                                    PmtDiscDueDate := WorkDate();
                            end else
                                PmtDiscDueDate := 0D;

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

PmtDiscDueDate := WorkDate()
else
PmtDiscDueDate := 0D;
end;
}
field(PmtDiscDueDate; PmtDiscDueDate)
{
ApplicationArea = Basic, Suite;
Caption = 'Payment Discount Date';
Editable = IncPmtDiscount;
ToolTip = 'Specifies the date that will be used to calculate the payment discount.';
}
field(MaximumAmount; MaximumAmount)
Expand Down Expand Up @@ -151,8 +160,10 @@ report 2000019 "Suggest Vendor Payments EB"
begin
if DueDate = 0D then
DueDate := WorkDate();
if PmtDiscDueDate = 0D then
PmtDiscDueDate := WorkDate();
if (IncPmtDiscount) and (PmtDiscDueDate = 0D) then

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\ 1}}$

Report 2000019 declares SaveValues = true, so IncPmtDiscount and PmtDiscDueDate are meant to persist across invocations.

The new OnOpenPage logic is if (IncPmtDiscount) and (PmtDiscDueDate = 0D) then PmtDiscDueDate := WorkDate() else PmtDiscDueDate := 0D;. When the checkbox was previously saved as true together with a non-zero discount date (the normal steady state after a first run), reopening the request page evaluates the second condition to false (PmtDiscDueDate <> 0D) and falls into the else branch, which wipes the persisted date back to 0D on every subsequent open. This defeats SaveValues for this field and also risks tripping the existing OnPreDataItem check if IncPmtDiscount and (PmtDiscDueDate < Today) then Confirm(...)/Error(...) on every run, since 0D < Today is always true. Actual impact is functionally significant (silent loss of a persisted user setting plus a spurious confirm/error each run), which would normally warrant major severity, but this is flagged as an unbacked agent finding and capped at minor per BCQuality's agent-finding rules; consider promoting this to a knowledge-backed rule. Recommended

Recommendation:

  • only default the date when it is currently blank, independent of clearing on uncheck, e.g. keep if PmtDiscDueDate = 0D then PmtDiscDueDate := WorkDate(); in OnOpenPage (mirroring DueDate/PostingDate) and confine the clear-on-uncheck behavior to the checkbox's own OnValidate trigger.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

PmtDiscDueDate := WorkDate()
else
PmtDiscDueDate := 0D;
if PostingDate = 0D then
PostingDate := WorkDate();
IncCreditMemos := true;
Expand Down
115 changes: 107 additions & 8 deletions src/Layers/BE/Tests/Local/ERMPaymentJournal.Codeunit.al
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,25 @@ codeunit 144003 "ERM Payment Journal"
end;

var
LibraryReportDataset: Codeunit "Library - Report Dataset";
LibraryRandom: Codeunit "Library - Random";
Assert: Codeunit Assert;
LibraryDimension: Codeunit "Library - Dimension";
LibraryERM: Codeunit "Library - ERM";
LibraryUtility: Codeunit "Library - Utility";
LibraryPurchase: Codeunit "Library - Purchase";
LibraryRandom: Codeunit "Library - Random";
LibraryReportDataset: Codeunit "Library - Report Dataset";
LibrarySales: Codeunit "Library - Sales";
Assert: Codeunit Assert;
LibrarySetupStorage: Codeunit "Library - Setup Storage";
LibraryTestInitialize: Codeunit "Library - Test Initialize";
LibraryUtility: Codeunit "Library - Utility";
LibraryVariableStorage: Codeunit "Library - Variable Storage";
LibraryDimension: Codeunit "Library - Dimension";
LibrarySetupStorage: Codeunit "Library - Setup Storage";
isInitialized: Boolean;
refExportProtocolType: Option Domestic,International;
IncorrectNumberOfDimErr: Label 'Incorrect number of dimensions.';
LettersTxt: Label 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', Locked = true;
WrongNumberOfLinesErr: Label 'Wrong number of Payment Journal Lines.';
WrongStatusOfLineErr: Label 'Wrong status of Payment Journal Line.';
WrongPmtDiscErr: Label 'Wrong %1 on the payment journal line.', Comment = '%1 = Field Caption';
WrongStatusOfBatchErr: Label 'Wrong status of Payment Journal Batch.';
LettersTxt: Label 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', Locked = true;
WrongStatusOfLineErr: Label 'Wrong status of Payment Journal Line.';

[Test]
[Scope('OnPrem')]
Expand Down Expand Up @@ -635,6 +636,76 @@ codeunit 144003 "ERM Payment Journal"
VerifyGenJnlLinesWithSameDescription(GenJnlBatch, PaymentJnlLine."Payment Message", 2);
end;

[Test]
[HandlerFunctions('SuggestVendorPaymentsTakeDiscRPH')]
procedure SuggestVendorPaymentsWithoutTakingPaymentDiscount()
var
PaymentJournalLine: Record "Payment Journal Line";
PurchaseHeader: Record "Purchase Header";
VendorLedgerEntry: Record "Vendor Ledger Entry";
VendorNo: Code[20];
begin
// [SCENARIO 637839] Payment discount is not taken when "Take Payment Discounts" is disabled in the "Suggest Vendor Payments EB" report.
Initialize();

// [GIVEN] Create a Vendor with Payment terms with Payment Discount.
VendorNo := CreateVendorWithPaymentDiscount();

// [GIVEN] Create and post a Purchase Invoice.
CreateAndPostPurchaseDocument(PurchaseHeader, PurchaseHeader."Document Type"::Invoice, VendorNo);

// [GIVEN] Find the Vendor Ledger Entry for the posted Purchase Invoice.
FindInvoiceVendorLedgerEntry(VendorLedgerEntry, VendorNo);

// [WHEN] Run "Suggest Vendor Payments EB" with "Take Payment Discounts" = FALSE.
LibraryVariableStorage.Enqueue(false);
SuggestVendorPayments(VendorNo);

// [WHEN] Filter the Payment Journal Line for the Vendor.
FilterEBPaymentJournalLine(PaymentJournalLine, VendorNo, PaymentJournalLine.Status::Created);
PaymentJournalLine.FindFirst();

// [THEN] Verify that the "Pmt. Discount Date" is Blank, Pmt Discount is not taken, in Payment Journal Line.
Assert.AreEqual(0D, PaymentJournalLine."Pmt. Discount Date", StrSubstNo(WrongPmtDiscErr, PaymentJournalLine."Pmt. Discount Date"));
Assert.AreEqual(0, PaymentJournalLine."Pmt. Disc. Possible", StrSubstNo(WrongPmtDiscErr, PaymentJournalLine."Pmt. Disc. Possible"));
LibraryVariableStorage.AssertEmpty();
end;

[Test]
[HandlerFunctions('SuggestVendorPaymentsTakeDiscRPH')]
procedure SuggestVendorPaymentsTakingPaymentDiscount()
var
PaymentJournalLine: Record "Payment Journal Line";
PurchaseHeader: Record "Purchase Header";
VendorLedgerEntry: Record "Vendor Ledger Entry";
VendorNo: Code[20];
begin
// [SCENARIO 637839] Payment discount is taken when "Take Payment Discounts" is enabled in the "Suggest Vendor Payments EB" report.
Initialize();

// [GIVEN] Create a Vendor with Payment terms with Payment Discount.
VendorNo := CreateVendorWithPaymentDiscount();

// [GIVEN] Create and post a Purchase Invoice.
CreateAndPostPurchaseDocument(PurchaseHeader, PurchaseHeader."Document Type"::Invoice, VendorNo);

// [GIVEN] Find the Vendor Ledger Entry for the posted Purchase Invoice.
FindInvoiceVendorLedgerEntry(VendorLedgerEntry, VendorNo);

// [WHEN] Run "Suggest Vendor Payments EB" with "Take Payment Discounts" = TRUE.
LibraryVariableStorage.Enqueue(true);
SuggestVendorPayments(VendorNo);

// [WHEN] Filter the Payment Journal Line for the Vendor.
FilterEBPaymentJournalLine(PaymentJournalLine, VendorNo, PaymentJournalLine.Status::Created);
PaymentJournalLine.FindFirst();

// [THEN] Verify that the "Pmt. Discount Date" and Pmt Discount is taken, in Payment Journal Line.
Assert.AreEqual(WorkDate(), PaymentJournalLine."Pmt. Discount Date", StrSubstNo(WrongPmtDiscErr, PaymentJournalLine."Pmt. Discount Date"));
Assert.AreEqual(PaymentJournalLine."Pmt. Disc. Possible", Abs(VendorLedgerEntry."Remaining Pmt. Disc. Possible"), StrSubstNo(WrongPmtDiscErr, PaymentJournalLine."Pmt. Disc. Possible"));
LibraryVariableStorage.AssertEmpty();
end;

local procedure Initialize()
begin
LibraryTestInitialize.OnTestInitialize(CODEUNIT::"ERM Payment Journal");
Expand Down Expand Up @@ -1488,6 +1559,26 @@ codeunit 144003 "ERM Payment Journal"
Assert.RecordCount(GenJournalLine, ExpectedCount);
end;

local procedure CreateVendorWithPaymentDiscount(): Code[20]
var
PaymentTerms: Record "Payment Terms";
Vendor: Record Vendor;
begin
LibraryERM.CreatePaymentTermsDiscount(PaymentTerms, false);
LibraryPurchase.CreateVendor(Vendor);
Vendor.Validate("Payment Terms Code", PaymentTerms.Code);
Vendor.Modify(true);

exit(Vendor."No.");
end;

local procedure FindInvoiceVendorLedgerEntry(var VendorLedgerEntry: Record "Vendor Ledger Entry"; VendorNo: Code[20])
begin
VendorLedgerEntry.SetRange("Document Type", VendorLedgerEntry."Document Type"::Invoice);
VendorLedgerEntry.SetRange("Vendor No.", VendorNo);
VendorLedgerEntry.FindFirst();
end;

[RequestPageHandler]
[Scope('OnPrem')]
procedure ExportPaymentJournalLinesHandler(var FileDomesticPayments: TestRequestPage "File Domestic Payments")
Expand Down Expand Up @@ -1532,5 +1623,13 @@ codeunit 144003 "ERM Payment Journal"
EBPaymentJournalTemplates.FILTER.SetFilter(Name, LibraryVariableStorage.DequeueText());
EBPaymentJournalTemplates.OK().Invoke();
end;

[RequestPageHandler]
procedure SuggestVendorPaymentsTakeDiscRPH(var SuggestVendorPaymentsEB: TestRequestPage "Suggest Vendor Payments EB")
begin
SuggestVendorPaymentsEB.DueDate.SetValue(CalcDate('<3M>', WorkDate()));
SuggestVendorPaymentsEB.IncPmtDiscount.SetValue(LibraryVariableStorage.DequeueBoolean());
SuggestVendorPaymentsEB.OK().Invoke();
end;
}

Loading