Recently, faced with a funny example of bad code in Base Application, I got the idea to write about this case and not only. This case illustrates how much work remains to be done in rewriting and improving our beloved Business Central product.
If you go to the Vendor Card and try to change the Vendor's name, this will trigger the table OnModify trigger, which will be our starting point.
Table 23 "Vendor", OnModify trigger
trigger OnModify()
begin
UpdateReferencedIds;
SetLastModifiedDateTime;
if IsContactUpdateNeeded then begin
Modify;
UpdateContFromVend.OnModify(Rec);
if not Find then begin
Reset;
if Find then;
end;
end;
end;
Let's skip the complex system of tracking record changes. This is possibly a topic for a separate conversation. Consider the UpdateContFromVend variable:
UpdateContFromVend: Codeunit "VendCont-Update";
VendCont-Update is a special codeunit that is responsible for synchronization between Vendor and Contact. It also contains a terrible line of code that caused the error on my project. I couldn't believe my eyes when I saw it.
procedure OnModify(var Vend: Record Vendor)
var
Cont: Record Contact;
OldCont: Record Contact;
ContBusRel: Record "Contact Business Relation";
ContNo: Code[20];
NoSeries: Code[20];
SalespersonCode: Code[20];
IsHandled: Boolean;
begin
with ContBusRel do begin
SetCurrentKey("Link to Table", "No.");
SetRange("Link to Table", "Link to Table"::Vendor);
SetRange("No.", Vend."No.");
if not FindFirst then
exit;
if not Cont.Get("Contact No.") then begin
Delete();
Session.LogMessage('0000B36', StrSubstNo(VendContactUpdateTelemetryMsg, "Contact No.", "Business Relation Code"), Verbosity::Normal, DataClassification::EndUserIdentifiableInformation, TelemetryScope::ExtensionPublisher, 'Category', VendContactUpdateCategoryTxt);
exit;
end;
OldCont := Cont;
end;
ContNo := Cont."No.";
NoSeries := Cont."No. Series";
SalespersonCode := Cont."Salesperson Code";
OnBeforeTransferFieldsFromVendToCont(Cont, Vend);
Cont.Validate("E-Mail", Vend."E-Mail");
Cont.TransferFields(Vend);
OnAfterTransferFieldsFromVendToCont(Cont, Vend);
IsHandled := false;
OnModifyOnBeforeAssignNo(Cont, IsHandled);
if not IsHandled then begin
Cont."No." := ContNo;
Cont."No. Series" := NoSeries;
end;
Cont."Salesperson Code" := SalespersonCode;
Cont.Validate(Name);
Cont.DoModify(OldCont);
Cont.Modify(true);
Vend.Get(Vend."No.");
OnAfterOnModify(Cont, OldCont, Vend);
end;
Seriously Cont.TransferFields(Vend); ? That is, we take all the matches according to the ID of our fields and try to copy them from Vendor to Contact. That is, the Vendor and Contact tables are now configured so that there are no ID conflicts when copying, lol :D
Of course, if you add fields via the table extension to Vendor and Contact with different types and the same IDs, it will cause an error. I think you often encountered such an error when posting documents, since the fields between table 36 "Sales Header" and, for example, table 112 "Sales Invoice Header" are also transferred using TransferFields function. But this is at least logical, in contrast to Vendor-Contact synchronization.
I think in this particular example there should be a simple assignment of the required fields, we should not follow the IDs of the fields during development. Even Microsoft shouldn't keep track of this for Vendor and Contact.
In the CreateItemFromTemplate procedure from "Item Card" page, the InsertItemFromTemplate function is called, but a completely empty local variable is passed. And this means that if the record was previously filtered, then if the templates are active, we lose these filters.
Page 30 "Item Card", CreateItemFromTemplate() function
local procedure CreateItemFromTemplate()
var
Item: Record Item;
InventorySetup: Record "Inventory Setup";
ItemTemplMgt: Codeunit "Item Templ. Mgt.";
begin
OnBeforeCreateItemFromTemplate(NewMode);
if not NewMode then
exit;
NewMode := false;
if ItemTemplMgt.InsertItemFromTemplate(Item) then begin
Copy(Item);
CurrPage.Update;
OnCreateItemFromTemplateOnAfterCurrPageUpdate(Rec);
end else
if ItemTemplMgt.TemplatesAreNotEmpty() then begin
CurrPage.Close;
exit;
end;
if ApplicationAreaMgmtFacade.IsFoundationEnabled then
if (Item."No." = '') and InventorySetup.Get then
Validate("Costing Method", InventorySetup."Default Costing Method");
end;
I suggest passing a record with filters to the procedure, or adding an additional parameter so that we can absolutely accurately determine the entry point when creating an item with a template.
I will now more closely monitor such design errors in Base Application, to report them and improve our Business Central.
Requests have been sent to Microsoft to improve these points:
https://github.com/microsoft/ALAppExtensions/issues/11649
https://github.com/microsoft/ALAppExtensions/issues/11026
Tested on BC 17.4.