-
Notifications
You must be signed in to change notification settings - Fork 0
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
[E-Document Core] - Extending the Core app with payments #5
base: main
Are you sure you want to change the base?
Conversation
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.
Great looking PR! I still have a few questions around the management CU pattern, but overall very nice, great work!
begin | ||
ErrorMessage.SetRange("Context Record ID", EDocument.RecordId()); | ||
ErrorMessage.SetRange("Table Number", Database::"E-Document Payment"); | ||
ErrorMessage.DeleteAll(); |
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.
Add explicit RunTrigger
|
||
/// <summary> | ||
/// This codeunit is used to implement the "Document Payment Handler" interface. It is used to provide a default implementation for the "Action Invoker" interface. | ||
/// </summary> |
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.
Action Invoker?
InherentPermissions = X; | ||
|
||
procedure Send(var EDocument: Record "E-Document"; var EDocumentService: Record "E-Document Service"; PaymentContext: Codeunit PaymentContext) | ||
begin |
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.
nit, preference: I prefer to see a // Do nothing comment within these placeholder implementations as it makes it clear this is intentionally empty.
|
||
trigger OnInsert() | ||
begin | ||
if Rec.Status = Rec.Status::" " then |
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.
Why even add the Empty status? Why not make Created the first one? or set the InitValue to Created?
using System.Utilities; | ||
|
||
/// <summary> | ||
/// Interface for sending and receiving paymnet information for E-Documents using E-Document service |
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.
Typo: ..payment..
Apps/W1/EDocument/app/src/Integration/Payments/PaymentIntegrationManagement.Codeunit.al
Show resolved
Hide resolved
FieldClass = FlowField; | ||
CalcFormula = sum("E-Document Payment".Amount where("E-Document Entry No." = field("Entry No"), Direction = field("Payment Direction Filter"))); | ||
} | ||
field(33; "Payment Direction Filter"; Enum "E-Document Direction") |
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 do not think we need to have Payment Direction Filter in the E-Document table. Related payments can be filtered based on Entry No. (I assume the document should not change direction :D ). Maybe we could save the direction only on E-Document Payment table.
{ | ||
Caption = 'Partially Paid'; | ||
} | ||
value(3; "Paid In Full") |
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.
nit: I would call it simple Paid
NotBlank = true; | ||
InitValue = 0T; | ||
} | ||
field(33; "Payment Sync Min between runs"; Integer) |
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.
Payment Sync Min Between Runs ?
@@ -43,21 +52,31 @@ table 6101 "E-Document Payment" | |||
{ | |||
Caption = 'VAT Base'; | |||
DecimalPlaces = 2; | |||
Editable = false; | |||
ToolTip = 'Specifies the value of the VAT Base field.'; |
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.
Let's add more meaningful tooltips.
Implementation The new functionality improves E-Document feature by introducing payment tracking. Users can now track both Incoming and Outgoing payments, with ability to monitor payment statuses by showing whether they are fully paid, partially paid or unpaid.
Default integration is a placeholder and does not actually send or receive payments. New integrations can be added by extending enum "Payment Integration" and implementing interface IDocumentPaymentHandler. |
if Rec."Paid Amount" < Rec."Amount Incl. VAT" then | ||
this.PaymentStatus := this.PaymentStatus::"Partially Paid" | ||
else | ||
this.PaymentStatus := this.PaymentStatus::Paid |
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.
this.PaymentStatus := this.PaymentStatus::Paid; ( ; is missing)
// ------------------------------------------------------------------------------------------------ | ||
namespace Microsoft.eServices.EDocument.Integration.Payments; | ||
|
||
|
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.
One empty line should be between namespace and using
Apps/W1/EDocument/app/src/Integration/Payments/PaymentContext.Codeunit.al
Show resolved
Hide resolved
PaymentStatus: Enum "Payment Status"; | ||
Date: Date; | ||
Amount: Decimal; | ||
|
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.
Remove the empty line before the global variables and object closing bracket
Payment.Date := PaymentContext.GetDate(); | ||
Payment.Validate(Amount, PaymentContext.GetAmount()); | ||
Payment.Status := PaymentContext.GetPaymentStatus(); | ||
Payment.Insert(); |
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.
Add explicit of RunTrigger
Implementation
The new functionality improves E-Document feature by introducing payment tracking. Users can now track both Incoming and Outgoing payments, with ability to monitor payment statuses by showing whether they are fully paid, partially paid or unpaid.
New field "Payment Integration" was added to E-Document Services page together with "Calculate Payment VAT" boolean. For the "Payment Integration" a user can set a particular payment handler/service for sending and receiving payments. By enabling "Calculate Payment VAT" boolean, payment amount is split into Base Amount and VAT Amount.
Additionally, the "Paid Amount" and "Payment Status" fields were added on E-Document page. On "Paid Amount" drilldown, user can access "E-Documents Payments" page, where payment date, paid amount without VAT and VAT amount are displayed, together with payment status and whether payment is incoming or outgoing. The payment date and amount fields can be edited manually, while the other fields are automatically populated by the system.
New actions "Receive Payments" and "Send Payment" have also been added under the "Actions" menu on the E-Document page. "Receive Payments" actions allows user to receive payments via service and all data related to payment appears on "E-Document Payment" page, as well as updated payment status on E-Document. Meanwhile, "Send Payment" action can be used to filter unsent payments based on status and direction. The system adds E-Document and its values, identifies the right service for sending and updates the payment status of E-Document. This occurs even when the payment is created manually, regardless of whether it has been sent yet.
Additional fields were added on "E-Documents Service" page, related to background job: "Auto Sync Payments", "Sync Start Time" and "Minutes between runs". The job queue is created when "Auto Sync Payments" boolean is enabled and same background job runs both sending and receiving actions.
Default integration is a placeholder and does not actually send or receive payments. New integrations can be added by extending enum "Payment Integration" and implementing interface IDocumentPaymentHandler.
MS PR: microsoft#27972