Volodymyr Dvernytskyi
Personal blog about Navision & Dynamics 365 Business Central

Bad code in Base Application.

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.

Strange TransferFields

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.

Creation of Item from template

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.

FRIDAY, March 12, 2021

Author: Volodymyr Dvernytskyi