Added dimension setup & posted entries for fixed assets#29390
Added dimension setup & posted entries for fixed assets#29390tuan-nguyen-fenwick wants to merge 26 commits intomicrosoft:mainfrom
Conversation
Excel report outputs showing meaningful data: - [Fixed Asset Projected Value (Excel).xlsx](https://dev.azure.com/fenwick/bf100862-1f7e-4467-bf86-526d4c7ac7eb/_apis/git/repositories/bc6a2808-2da7-4039-8a77-6283538e4d5d/pullRequests/38641/attachments/Fixed%20Asset%20Projected%20Value%20%28Excel%29.xlsx) - [Fixed Asset Details (Excel).xlsx](https://dev.azure.com/fenwick/bf100862-1f7e-4467-bf86-526d4c7ac7eb/_apis/git/repositories/bc6a2808-2da7-4039-8a77-6283538e4d5d/pullRequests/38641/attachments/Fixed%20Asset%20Details%20%28Excel%29.xlsx) - [Fixed Asset Analysis (Excel).xlsx](https://dev.azure.com/fenwick/bf100862-1f7e-4467-bf86-526d4c7ac7eb/_apis/git/repositories/bc6a2808-2da7-4039-8a77-6283538e4d5d/pullRequests/38641/attachments/Fixed%20Asset%20Analysis%20%28Excel%29.xlsx) FA Ledger Entries:  Published to: http://bcdev2.fenwicksoftware.com.au:10320/BC/?company=Demo21-FA Related work items: microsoft#28277
...ContosoCoffeeDemoDataset/app/DemoData/FixedAsset/3.Transactions/CreateFAJnlLines.Codeunit.al
Outdated
Show resolved
Hide resolved
|
Processing this PR. The branch is now locked 🔒 Please don't push updates unless otherwise agreed. |
|
Another PR where the local DBs don't build any longer. This time with the following: We need to understand what's happening. But that'll be a job for 2026 😉 |
Hi @JesperSchulz, Happy New Year! Can you share which region(s) throwed this error please? |
BuildDatabase_CZ, BuildDatabase_DK and BuildDatabase_NL. And happy new year 🥳 |
|
Let me try again! |
|
NL build failed: |
@JesperSchulz Thanks, it should be fixed now. I corrected the namespace usage. |
|
@JesperSchulz Please let me know if there’s anything else needed to wrap up this PR. Thanks! |
I'll try running it again with the latest chages. |
|
DK now fails with the following: |
|
Hello @tuan-nguyen-fenwick Do you have enough information in the previous message to fix the build error? |
|
@JesperSchulz @attilatoury I've resolved the FA journal posting error in DK. BC docker version: DK Business Central 28.0 (Platform 28.0.44791.0 + Application 28.0.44914.0)
|
|
Giving this another go! |
|
@tuan-nguyen-fenwick The build succeeded but I am adding a few comments from our internal code review |
| var | ||
| CreateGLAccount: Codeunit "Create G/L Account"; | ||
| begin | ||
| OnBeforeGetCashAccountForFixedAssetAcquisition(GLAccountNo); |
There was a problem hiding this comment.
Those individual events will soon make the codebase difficult to maintain, to see how we localize GL accounts, see the coding patterns in this file (under the section GL Account):
https://github.com/microsoft/ALAppExtensions/blob/main/Apps/W1/ContosoCoffeeDemoDataset/app/Coding-Patterns.md
There was a problem hiding this comment.
Thank you, I've restructured and changed to using more generic event OnBeforeInsertEvent for CZ, NL and DK instead of relying on a custom event.
| Codeunit.Run(Codeunit::"Post FA Jnl. Lines"); | ||
| end; | ||
|
|
||
| procedure InsertFAGenJournalLine(JournalTemplateName: Code[10]; JournalBatchName: Code[10]; LineNo: Integer; AccountNo: Code[20]; PostingDate: Date; FAPostingType: Enum "Gen. Journal Line FA Posting Type"; DocumentNo: Code[20]; Description: Text[100]; BalAccountType: Enum "Gen. Journal Account Type"; BalAccountNo: Code[20]; Amount: Decimal) |
There was a problem hiding this comment.
This type of function should be in the helper codeunit
There was a problem hiding this comment.
I've moved it to Contoso Fixed Assets helper codeunit
| InsertFAGenJournalLine(FAJournalTemplateName, FAJournalBatchName, 60000, CreateFixedAsset.FA000070(), ContosoUtilities.AdjustDate(19010101D), Enum::"Gen. Journal Line FA Posting Type"::"Acquisition Cost", AssetAcquisitionLbl, AcquisitionLbl, Enum::"Gen. Journal Account Type"::"G/L Account", BalanceAccountNo, 1500); | ||
| InsertFAGenJournalLine(FAJournalTemplateName, FAJournalBatchName, 70000, CreateFixedAsset.FA000080(), ContosoUtilities.AdjustDate(19010101D), Enum::"Gen. Journal Line FA Posting Type"::"Acquisition Cost", AssetAcquisitionLbl, AcquisitionLbl, Enum::"Gen. Journal Account Type"::"G/L Account", BalanceAccountNo, 11000); | ||
| InsertFAGenJournalLine(FAJournalTemplateName, FAJournalBatchName, 80000, CreateFixedAsset.FA000090(), ContosoUtilities.AdjustDate(19010101D), Enum::"Gen. Journal Line FA Posting Type"::"Acquisition Cost", AssetAcquisitionLbl, AcquisitionLbl, Enum::"Gen. Journal Account Type"::"G/L Account", BalanceAccountNo, 4000); | ||
| Codeunit.Run(Codeunit::"Post FA Jnl. Lines"); |
There was a problem hiding this comment.
The posting of documents is usually done in a codeunit under the folder 4.Historical
There was a problem hiding this comment.
@attilatoury I have moved these under Historical folder.
…etAcquisition event
|
All changes merged to internal PR. Running CI... |
| end; | ||
|
|
||
| [EventSubscriber(ObjectType::Table, Database::"Gen. Journal Line", OnBeforeInsertEvent, '', false, false)] | ||
| local procedure OnBeforeInsertGenJournalLineDK(var Rec: Record "Gen. Journal Line") |
There was a problem hiding this comment.
This subscribes to an event from a BaseApp table and will be executed too many times without being used.
Also, we really want to limit the use of events in these demo data app in general.
If the goal is to use the value from the DK GL account instead of the one from W1, this is what the localization of GL Account is already guaranteeing us: https://github.com/microsoft/ALAppExtensions/blob/main/Apps/DK/ContosoCoffeeDemoDatasetDK/app/DemoData/Finance/1.Setup%20Data/CreateGLAccDK.Codeunit.al
That means that all the inserts done in W1 using W1's GL accounts will actually be using DK's GL accounts (if they have been overwritten in the codeunit linked above).
For specific scenarios where the W1 GL account cannot be localized (e.g. the account only exists in the localization), then one option is to find the data that was inserted in W1, and either delete it and insert it with the localization's values, or modify it.
Hopefully the Contoso Coffee Demo Dataset apps (W1 and all the localizations) provide extensive examples on how to do it.
Note that this also applies to other localized files where this pattern is used.
There was a problem hiding this comment.
@attilatoury Thanks for the suggestions.
Just wanted to clarify the above G/L account creation pattern and my PR scenario. This "Create G/L " codeunit is designed to create localized G/L accounts which I totally understand.
In my case, I want to call the localized account that's already been created for the specific country and use it on the FA journal lines. I'm not creating any new account for DK.
For example, W1 cash account = 1234, whereas DK cash account = 6789, my code here calls the DK cash account (6789) and populate on the FA journal lines when generating Fixed Assets data only. The codeunit itself is manual binding so it isn't triggered outside the scope of creating Fixed Assets demo data module.
There was a problem hiding this comment.
@tuan-nguyen-fenwick If the account has already been localized, then calling the W1 account will return the DK account (when called from the DK localized app). So, intercepting the data before it is inserted to overwrite the account number should not be needed.
There was a problem hiding this comment.
You're right, calling the W1 cash account CreateGLAccount.Cash() works for all countries except for DK, NL and CZ. It returns a blank value because these countries have different name and account number for the Cash account. And when the balancing account is blank on the Gen. Jnl. Line, it causes out of balance during posting.
And for that reason, I had to call the specific localized account in DK, NL and CZ.
Let me know your thoughts to get this PR moving.
There was a problem hiding this comment.
@tuan-nguyen-fenwick In that case, the pattern we use for demo data is "For specific scenarios where the W1 GL account cannot be localized (e.g. the account only exists in the localization), then one option is to find the data that was inserted in W1, and either delete it and insert it with the localization's values, or modify it."
There was a problem hiding this comment.
@attilatoury
For example in CZ, it is already been done here in the account localization codeunit where it clears W1 Cash account number (that's same as intercepting and deleting it from CZ environment) and then replaces it the localized account.
There was a problem hiding this comment.
@tuan-nguyen-fenwick Right so can you use the same pattern for the other countries that you are modifying?
There was a problem hiding this comment.
@attilatoury Since we're approaching the v28 branching date, I don’t think it’s the right time to change how the localized Cash account is created for these countries in this PR. Adjusting this behaviour will require more planning as it may impact other areas of the local Demo Data apps.
There may also be valid functional reasons behind why Cash account is set up differently in these countries, so this needs a broader review if we proceed.


Summary
Added dimension setup and posted entries for Fixed Assets in Demo Data Tool.
Work Item(s)
Fixes #29388
Fixes AB#597245