Discussion: Introduce company initialize in System Application #2317
Conversation
|
Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234' |
| } | ||
| field(3; "Initialized Version"; Code[20]) | ||
| { | ||
| } |
There was a problem hiding this comment.
One thing I'd add here is a field: IsCompanyInitialized (or something like that)
If you look at some code in BaseApp, we are leveraging the existences of GLSetup to determine this
There was a problem hiding this comment.
Yes, I was using the fact that the table is empty as it not being initialized. Do you have a suggestion for when we want to insert a record but state that the company has not been initialized?
| CompanyInitializeCodeunit.InitializeCompanySetup(); | ||
|
|
||
| CompanyInitializeCodeunit.InitializeCompany(); |
There was a problem hiding this comment.
What's the gain of raising two identical events (except for their name) in a row?
And shouldn't their names start with On*?
There was a problem hiding this comment.
True, they should start with On. My thought on two events is that we have a lot of setup tables that are standalone and doesn't depend on anything else being setup and then we have other tables that does depend on the initial setup tables being filled in. With this you always subscribe to the first event if you're independent of any other setup and second if you are dependent. Of course this does not account for third or fourth level dependencies and there are also many other ways to deal with this kind of problem. It's simply trying to help a very complicated problem a little bit.
| CompanyInitializeCodeunit.InitializeCompany(); | ||
| end; | ||
|
|
||
| [EventSubscriber(ObjectType::Codeunit, Codeunit::"System Initialization", 'OnAfterLogin', '', false, false)] |
There was a problem hiding this comment.
| [EventSubscriber(ObjectType::Codeunit, Codeunit::"System Initialization", 'OnAfterLogin', '', false, false)] | |
| [EventSubscriber(ObjectType::Codeunit, Codeunit::"System Initialization", OnAfterLogin, '', false, false)] |
|
|
||
| namespace System.Environment.Configuration; | ||
|
|
||
| codeunit 900 "Company Initialize" |
There was a problem hiding this comment.
This should be moved to the platform system events. Whenever a new company is created they should trigger the initialize company event.
There was a problem hiding this comment.
This is a nice opportunity to improve the flows.
|
|
||
| fields | ||
| { | ||
| field(1; "Primary Key"; Integer) |
There was a problem hiding this comment.
This should be moved to the company table so we track for each company.
Keep in mind that it is by design that we can run it multiple times.
There was a problem hiding this comment.
Can you explain why we want to run this multiple times?
|
|
||
| if NavApp.GetCurrentModuleInfo(ModuleInfo) then | ||
| CompanyInitialize."Initialized Version" := format(ModuleInfo.AppVersion()); | ||
| CompanyInitialize.Insert(); |
There was a problem hiding this comment.
We can initialize company many times, the codeunit 2 must be safe to run many times. This logic is also wrong, it should be moved after Initialize company not before. We could partially commit and keep the record.
| codeunit 900 "Company Initialize" | ||
| { | ||
| [IntegrationEvent(false, false)] | ||
| internal procedure InitializeCompanySetup() |
There was a problem hiding this comment.
I prefer On for the events, it increases readability
| if not GuiAllowed() then | ||
| exit; | ||
|
|
||
| if ClientTypeManagement.GetCurrentClientType() = ClientType::Background then |
There was a problem hiding this comment.
The most important check that needs to be refactored is the check that we have on the GLSetup.Get(). We should introduce concepts of force initialization and a better check if it was implemented or not.
I have created this PR to start a discussion of whether we should have company initialize in System Application and see how it might look like. We currently have apps with tables that require setup data per company. So far we insert data on the fly, but should we always do this check or should it be done as part of initializing each company?
Either way, let's see.
System Application examples are: "AAD Application Setup", "Setup Azure AD Mgt. Provider", "Register Company Signal".