-
Notifications
You must be signed in to change notification settings - Fork 687
Added dimension setup & posted entries for fixed assets #29390
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
base: main
Are you sure you want to change the base?
Added dimension setup & posted entries for fixed assets #29390
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type of function should be in the helper codeunit
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'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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The posting of documents is usually done in a codeunit under the folder 4.Historical
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.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.


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